2
0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 14:07:59 +00:00

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

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;
}
This commit is contained in:
Ondřej Surý 2019-09-27 10:00:46 +02:00
parent 91cc6b9eb9
commit d8879af877

View File

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