Fix passing NULL after the last typed argument to a variadic function leads to undefi...

From Cppcheck:

Passing NULL after the last typed argument to a variadic function leads to
undefined behaviour.  The C99 standard, in section 7.15.1.1, states that if the
type used by va_arg() is not compatible with the type of the actual next
argument (as promoted according to the default argument promotions), the
behavior is undefined.  The value of the NULL macro is an implementation-defined
null pointer constant (7.17), which can be any integer constant expression with
the value 0, or such an expression casted to (void*) (6.3.2.3). This includes
values like 0, 0L, or even 0LL.In practice on common architectures, this will
cause real crashes if sizeof(int) != sizeof(void*), and NULL is defined to 0 or
any other null pointer constant that promotes to int.  To reproduce you might be
able to use this little code example on 64bit platforms. If the output includes
"ERROR", the sentinel had only 4 out of 8 bytes initialized to zero and was not
detected as the final argument to stop argument processing via
va_arg(). Changing the 0 to (void*)0 or 0L will make the "ERROR" output go away.

void f(char *s, ...) {
    va_list ap;
    va_start(ap,s);
    for (;;) {
        char *p = va_arg(ap,char*);
        printf("%018p, %s\n", p, (long)p & 255 ? p : "");
        if(!p) break;
    }
    va_end(ap);
}

void g() {
    char *s2 = "x";
    char *s3 = "ERROR";

    // changing 0 to 0L for the 7th argument (which is intended to act as
    // sentinel) makes the error go away on x86_64
    f("first", s2, s2, s2, s2, s2, 0, s3, (char*)0);
}

void h() {
    int i;
    volatile unsigned char a[1000];
    for (i = 0; i<sizeof(a); i++)
        a[i] = -1;
}

int main() {
    h();
    g();
    return 0;
}

diff --git a/lib/dns/geoip2.c b/lib/dns/geoip2.c
index 513e423fa5c9f3812dd42582b4fc0ebb828a4949..c0ab66a2415c3220506b1bbcd05cf351d53ec105 100644 (file)
--- a/lib/dns/geoip2.c
+++ b/lib/dns/geoip2.c
@@ -326,7 +326,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        geoip_state_t *state = NULL;
        dns_geoip_subtype_t subtype;
        const char *s = NULL;
-       int ret, i;
+       int ret;
 
        REQUIRE(reqaddr != NULL);
        REQUIRE(elt != NULL);
@@ -347,7 +347,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_country_code:
        case dns_geoip_city_countrycode:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "country", "iso_code", NULL);
+                                    "country", "iso_code", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -356,7 +356,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_country_name:
        case dns_geoip_city_countryname:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "country", "names", "en", NULL);
+                                    "country", "names", "en", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -365,7 +365,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_country_continentcode:
        case dns_geoip_city_continentcode:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "continent", "code", NULL);
+                                    "continent", "code", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -374,7 +374,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_country_continent:
        case dns_geoip_city_continent:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "continent", "names", "en", NULL);
+                                    "continent", "names", "en", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -383,7 +383,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_region:
        case dns_geoip_city_region:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "subdivisions", "0", "iso_code", NULL);
+                                    "subdivisions", "0", "iso_code", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -392,7 +392,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
        case dns_geoip_regionname:
        case dns_geoip_city_regionname:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "subdivisions", "0", "names", "en", NULL);
+                                    "subdivisions", "0", "names", "en", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -400,7 +400,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
 
        case dns_geoip_city_name:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "city", "names", "en", NULL);
+                                    "city", "names", "en", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -408,7 +408,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
 
        case dns_geoip_city_postalcode:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "postal", "code", NULL);
+                                    "postal", "code", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -416,7 +416,7 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
 
        case dns_geoip_city_timezonecode:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "location", "time_zone", NULL);
+                                    "location", "time_zone", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -425,14 +425,14 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
 
        case dns_geoip_city_metrocode:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "location", "metro_code", NULL);
+                                    "location", "metro_code", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
                break;
 
        case dns_geoip_isp_name:
-               ret = MMDB_get_value(&state->entry, &value, "isp", NULL);
+               ret = MMDB_get_value(&state->entry, &value, "isp", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
@@ -442,8 +442,9 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
                INSIST(elt->as_string != NULL);
 
                ret = MMDB_get_value(&state->entry, &value,
-                                    "autonomous_system_number", NULL);
+                                    "autonomous_system_number", (char *)0);
                if (ret == MMDB_SUCCESS) {
+                       int i;
                        s = elt->as_string;
                        if (strncasecmp(s, "AS", 2) == 0) {
                                s += 2;
@@ -455,14 +456,14 @@ dns_geoip_match(const isc_netaddr_t *reqaddr,
 
        case dns_geoip_org_name:
                ret = MMDB_get_value(&state->entry, &value,
-                                    "autonomous_system_organization", NULL);
+                                    "autonomous_system_organization", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }
                break;
 
        case dns_geoip_domain_name:
-               ret = MMDB_get_value(&state->entry, &value, "domain", NULL);
+               ret = MMDB_get_value(&state->entry, &value, "domain", (char *)0);
                if (ret == MMDB_SUCCESS) {
                        return (match_string(&value, elt->as_string));
                }