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;
}

(cherry picked from commit d8879af877c232bd15d0011e65c8759f36a09901)

diff --git a/lib/dns/geoip2.c b/lib/dns/geoip2.c
index 3a462036f6fe79a028a358aee64a10200c78301c..7854e6db96a37453d72360a242e24ef427c86fa0 100644 (file)
--- a/lib/dns/geoip2.c
+++ b/lib/dns/geoip2.c
@@ -333,7 +333,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);
@@ -354,7 +354,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));
                }
@@ -363,7 +363,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));
                }
@@ -372,7 +372,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));
                }
@@ -381,7 +381,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));
                }
@@ -390,7 +390,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));
                }
@@ -399,7 +399,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));
                }
@@ -407,7 +407,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));
                }
@@ -415,7 +415,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));
                }
@@ -423,7 +423,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));
                }
@@ -432,14 +432,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));
                }
@@ -449,8 +449,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;
@@ -462,14 +463,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));
                }