From beaca3e8dbdafcf7474ff07c8f95116919367ac2 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 9 Mar 2020 18:23:13 -0700 Subject: [PATCH 1/4] remove or comment empty conditional branches some empty conditional branches which contained a semicolon were "fixed" by clang-format to contain nothing. add comments to prevent this. (cherry picked from commit 735be3b816913f8fae3153366ec89e5511ff52b4) --- bin/named/zoneconf.c | 4 ++-- lib/dns/rdata/in_1/wks_11.c | 39 ++++++++++++++++++++++--------------- 2 files changed, 25 insertions(+), 18 deletions(-) diff --git a/bin/named/zoneconf.c b/bin/named/zoneconf.c index 6a28cf83ae..e01b095bb4 100644 --- a/bin/named/zoneconf.c +++ b/bin/named/zoneconf.c @@ -1442,7 +1442,7 @@ ns_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, } else if (strcasecmp(arg, "maintain") == 0) { allow = maint = true; } else if (strcasecmp(arg, "off") == 0) { - ; + /* Default */ } else { INSIST(0); ISC_UNREACHABLE(); @@ -1591,7 +1591,7 @@ ns_zone_configure(const cfg_obj_t *config, const cfg_obj_t *vconfig, dns_zone_setkeyopt(zone, DNS_ZONEKEY_NORESIGN, true); } else if (strcasecmp(arg, "maintain") == 0) { - ; + /* Default */ } else { INSIST(0); ISC_UNREACHABLE(); diff --git a/lib/dns/rdata/in_1/wks_11.c b/lib/dns/rdata/in_1/wks_11.c index 8ad0bf6ea1..ed77c2f2d0 100644 --- a/lib/dns/rdata/in_1/wks_11.c +++ b/lib/dns/rdata/in_1/wks_11.c @@ -73,7 +73,7 @@ fromtext_in_wks(ARGS_FROMTEXT) { isc_token_t token; isc_region_t region; struct in_addr addr; - char *e; + char *e = NULL; long proto; unsigned char bm[8*1024]; /* 64k bits */ long port; @@ -115,10 +115,12 @@ fromtext_in_wks(ARGS_FROMTEXT) { false)); isc_buffer_availableregion(target, ®ion); - if (getquad(DNS_AS_STR(token), &addr, lexer, callbacks) != 1) + if (getquad(DNS_AS_STR(token), &addr, lexer, callbacks) != 1) { CHECKTOK(DNS_R_BADDOTTEDQUAD); - if (region.length < 4) + } + if (region.length < 4) { return (ISC_R_NOSPACE); + } memmove(region.base, &addr, 4); isc_buffer_add(target, 4); @@ -129,18 +131,19 @@ fromtext_in_wks(ARGS_FROMTEXT) { false)); proto = strtol(DNS_AS_STR(token), &e, 10); - if (*e == 0) - ; - else if (!mygetprotobyname(DNS_AS_STR(token), &proto)) + if (*e != '\0' && !mygetprotobyname(DNS_AS_STR(token), &proto)) { CHECKTOK(DNS_R_UNKNOWNPROTO); + } - if (proto < 0 || proto > 0xff) + if (proto < 0 || proto > 0xff) { CHECKTOK(ISC_R_RANGE); + } - if (proto == IPPROTO_TCP) + if (proto == IPPROTO_TCP) { ps = "tcp"; - else if (proto == IPPROTO_UDP) + } else if (proto == IPPROTO_UDP) { ps = "udp"; + } CHECK(uint8_tobuffer(proto, target)); @@ -148,8 +151,9 @@ fromtext_in_wks(ARGS_FROMTEXT) { do { CHECK(isc_lex_getmastertoken(lexer, &token, isc_tokentype_string, true)); - if (token.type != isc_tokentype_string) + if (token.type != isc_tokentype_string) { break; + } /* * Lowercase the service string as some getservbyname() are @@ -161,15 +165,18 @@ fromtext_in_wks(ARGS_FROMTEXT) { service[i] = tolower(service[i]&0xff); port = strtol(DNS_AS_STR(token), &e, 10); - if (*e == 0) - ; - else if (!mygetservbyname(service, ps, &port) && - !mygetservbyname(DNS_AS_STR(token), ps, &port)) + if (*e != 0 && !mygetservbyname(service, ps, &port) && + !mygetservbyname(DNS_AS_STR(token), ps, &port)) + { CHECKTOK(DNS_R_UNKNOWNSERVICE); - if (port < 0 || port > 0xffff) + } + + if (port < 0 || port > 0xffff) { CHECKTOK(ISC_R_RANGE); - if (port > maxport) + } + if (port > maxport) { maxport = port; + } bm[port / 8] |= (0x80 >> (port % 8)); } while (1); From c16c095b32d57a484abd2a0918ef36f65c64cbbc Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 9 Mar 2020 18:52:33 -0700 Subject: [PATCH 2/4] replace unsafe ctime() and gmtime() function calls This silences LGTM warnings that these functions are not thread-safe. (cherry picked from commit 5703f704273955f6c54627b6e05d8eb009b81337) --- bin/dnssec/dnssec-settime.c | 18 ++++++++++++++---- bin/dnssec/dnssec-signzone.c | 14 +++++++++++--- lib/dns/gen.c | 7 ++++++- 3 files changed, 31 insertions(+), 8 deletions(-) diff --git a/bin/dnssec/dnssec-settime.c b/bin/dnssec/dnssec-settime.c index 7afcaee096..62ef8422c7 100644 --- a/bin/dnssec/dnssec-settime.c +++ b/bin/dnssec/dnssec-settime.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -109,7 +110,6 @@ printtime(dst_key_t *key, int type, const char *tag, bool epoch, FILE *stream) { isc_result_t result; - const char *output = NULL; isc_stdtime_t when; if (tag != NULL) @@ -121,9 +121,19 @@ printtime(dst_key_t *key, int type, const char *tag, bool epoch, } else if (epoch) { fprintf(stream, "%d\n", (int) when); } else { - time_t timet = when; - output = ctime(&timet); - fprintf(stream, "%s", output); + time_t now = (time_t)when; +#ifdef _MSC_VER + struct tm *tm = localtime(&now); /* Thread specific. */ +#else + struct tm t, *tm = localtime_r(&now, &t); +#endif + unsigned int flen; + char timebuf[80]; + + flen = strftime(timebuf, sizeof(timebuf), + "%a %b %e %H:%M:%S %Y", tm); + INSIST(flen > 0U && flen < sizeof(timebuf)); + fprintf(stream, "%s\n", timebuf); } } diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 319a8050bd..8d93be83fa 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -2954,13 +2954,21 @@ writeset(const char *prefix, dns_rdatatype_t type) { static void print_time(FILE *fp) { - time_t currenttime; + time_t currenttime = time(NULL); +#ifdef _MSC_VER + struct tm *tm = localtime(¤ttime); /* Thread specific. */ +#else + struct tm t, *tm = localtime_r(¤ttime, &t); +#endif + unsigned int flen; + char timebuf[80]; if (outputformat != dns_masterformat_text) return; - currenttime = time(NULL); - fprintf(fp, "; File written on %s", ctime(¤ttime)); + flen = strftime(timebuf, sizeof(timebuf), "%a %b %e %H:%M:%S %Y", tm); + INSIST(flen > 0U && flen < sizeof(timebuf)); + fprintf(fp, "; File written on %s\n", timebuf); } static void diff --git a/lib/dns/gen.c b/lib/dns/gen.c index 5fbdc3bfd3..589cd20526 100644 --- a/lib/dns/gen.c +++ b/lib/dns/gen.c @@ -689,7 +689,12 @@ main(int argc, char **argv) { } if (now != -1) { - struct tm *tm = gmtime(&now); +#ifdef _MSC_VER + struct tm *tm = gmtime(&now); /* Thread specific. */ +#else + struct tm t, *tm = gmtime_r(&now, &t); +#endif + if (tm != NULL && tm->tm_year > 104) { n = snprintf(year, sizeof(year), "-%d", tm->tm_year + 1900); From bdb8e3ad856efa0cf71d7a5dc7c9ccaff46994a0 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 9 Mar 2020 20:51:21 -0700 Subject: [PATCH 3/4] silence a warning about unsafe snprintf() call (cherry picked from commit ec95b84e8d2f0e406cd15282f5b31fba7b3e84ce) --- lib/dns/rpz.c | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index ec8dc37618..375d2bf9c5 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -696,13 +696,14 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, (tgt_ip->w[3]>>8) & 0xffU, (tgt_ip->w[3]>>16) & 0xffU, (tgt_ip->w[3]>>24) & 0xffU); - if (len < 0 || len > (int)sizeof(str)) { + if (len < 0 || (size_t)len >= sizeof(str)) { return (ISC_R_FAILURE); } } else { len = snprintf(str, sizeof(str), "%d", tgt_prefix); - if (len == -1) + if (len < 0 || (size_t)len >= sizeof(str)) { return (ISC_R_FAILURE); + } for (i = 0; i < DNS_RPZ_CIDR_WORDS; i++) { w[i*2+1] = ((tgt_ip->w[DNS_RPZ_CIDR_WORDS-1-i] >> 16) & 0xffff); @@ -732,15 +733,19 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, } for (n = 0; n <= 7; ++n) { - INSIST(len < (int)sizeof(str)); + INSIST(len > 0 && (size_t)len < sizeof(str)); if (n == best_first) { - len += snprintf(str + len, sizeof(str) - len, - ".zz"); + i = snprintf(str + len, sizeof(str) - len, + ".zz"); n += best_len - 1; } else { - len += snprintf(str + len, sizeof(str) - len, - ".%x", w[n]); + i = snprintf(str + len, sizeof(str) - len, + ".%x", w[n]); } + if (i < 0 || (size_t)i >= (size_t)(sizeof(str) - len)) { + return (ISC_R_FAILURE); + } + len += i; } } From 5ae4d3d94a0144ee5aa65c9c48138b0dca9a57a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 13 Mar 2020 08:38:37 +0100 Subject: [PATCH 4/4] Add C11 localtime_r and gmtime_r shims for Windows On Windows, C11 localtime_r() and gmtime_r() functions are not available. While localtime() and gmtime() functions are already thread safe because they use Thread Local Storage, it's quite ugly to #ifdef around every localtime_r() and gmtime_r() usage to make the usage also thread-safe on POSIX platforms. The commit adds wrappers around Windows localtime_s() and gmtime_s() functions. NOTE: The implementation of localtime_s and gmtime_s in Microsoft CRT are incompatible with the C standard since it has reversed parameter order and errno_t return type. (cherry picked from commit 08f4c7d6c094c93ce7df093a8aa0890618959c83) --- bin/dig/dig.c | 4 ---- bin/dnssec/dnssec-settime.c | 11 ++++++----- bin/dnssec/dnssec-signzone.c | 7 ++----- lib/dns/gen-win32.h | 11 +++++++++++ lib/dns/gen.c | 4 ---- lib/dns/update.c | 16 ++++++---------- lib/isc/win32/include/isc/time.h | 25 +++++++++++++++++++++++++ 7 files changed, 50 insertions(+), 28 deletions(-) diff --git a/bin/dig/dig.c b/bin/dig/dig.c index 706299e75a..996cbb9495 100644 --- a/bin/dig/dig.c +++ b/bin/dig/dig.c @@ -277,11 +277,7 @@ received(unsigned int bytes, isc_sockaddr_t *from, dig_query_t *query) { printf(";; Query time: %ld msec\n", (long) diff / 1000); printf(";; SERVER: %s(%s)\n", fromtext, query->servname); time(&tnow); -#if defined(ISC_PLATFORM_USETHREADS) && !defined(WIN32) (void)localtime_r(&tnow, &tmnow); -#else - tmnow = *localtime(&tnow); -#endif #ifdef WIN32 /* diff --git a/bin/dnssec/dnssec-settime.c b/bin/dnssec/dnssec-settime.c index 62ef8422c7..e128ee7b4b 100644 --- a/bin/dnssec/dnssec-settime.c +++ b/bin/dnssec/dnssec-settime.c @@ -121,15 +121,16 @@ printtime(dst_key_t *key, int type, const char *tag, bool epoch, } else if (epoch) { fprintf(stream, "%d\n", (int) when); } else { - time_t now = (time_t)when; -#ifdef _MSC_VER - struct tm *tm = localtime(&now); /* Thread specific. */ -#else + time_t now = when; struct tm t, *tm = localtime_r(&now, &t); -#endif unsigned int flen; char timebuf[80]; + if (tm == NULL) { + fprintf(stream, "INVALID\n"); + return; + } + flen = strftime(timebuf, sizeof(timebuf), "%a %b %e %H:%M:%S %Y", tm); INSIST(flen > 0U && flen < sizeof(timebuf)); diff --git a/bin/dnssec/dnssec-signzone.c b/bin/dnssec/dnssec-signzone.c index 8d93be83fa..cd235b2e8f 100644 --- a/bin/dnssec/dnssec-signzone.c +++ b/bin/dnssec/dnssec-signzone.c @@ -2955,16 +2955,13 @@ writeset(const char *prefix, dns_rdatatype_t type) { static void print_time(FILE *fp) { time_t currenttime = time(NULL); -#ifdef _MSC_VER - struct tm *tm = localtime(¤ttime); /* Thread specific. */ -#else struct tm t, *tm = localtime_r(¤ttime, &t); -#endif unsigned int flen; char timebuf[80]; - if (outputformat != dns_masterformat_text) + if (tm == NULL || outputformat != dns_masterformat_text) { return; + } flen = strftime(timebuf, sizeof(timebuf), "%a %b %e %H:%M:%S %Y", tm); INSIST(flen > 0U && flen < sizeof(timebuf)); diff --git a/lib/dns/gen-win32.h b/lib/dns/gen-win32.h index 4791032650..18a98950c3 100644 --- a/lib/dns/gen-win32.h +++ b/lib/dns/gen-win32.h @@ -62,6 +62,7 @@ #include #include #include +#include #include #include @@ -268,6 +269,16 @@ end_directory(isc_dir_t *dir) { FindClose(dir->handle); } +inline struct tm * +gmtime_r(const time_t *clock, struct tm *result) { + errno_t ret = gmtime_s(result, clock); + if (ret != 0) { + errno = ret; + return (NULL); + } + return (result); +} + ISC_LANG_ENDDECLS #endif /* DNS_GEN_WIN32_H */ diff --git a/lib/dns/gen.c b/lib/dns/gen.c index 589cd20526..fe6fef0de2 100644 --- a/lib/dns/gen.c +++ b/lib/dns/gen.c @@ -689,11 +689,7 @@ main(int argc, char **argv) { } if (now != -1) { -#ifdef _MSC_VER - struct tm *tm = gmtime(&now); /* Thread specific. */ -#else struct tm t, *tm = gmtime_r(&now, &t); -#endif if (tm != NULL && tm->tm_year > 104) { n = snprintf(year, sizeof(year), "-%d", diff --git a/lib/dns/update.c b/lib/dns/update.c index f735642991..085cc5509c 100644 --- a/lib/dns/update.c +++ b/lib/dns/update.c @@ -2082,16 +2082,12 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db, static isc_stdtime_t epoch_to_yyyymmdd(time_t when) { - struct tm *tm; - -#if defined(ISC_PLATFORM_USETHREADS) && !defined(WIN32) - struct tm tm0; - tm = localtime_r(&when, &tm0); -#else - tm = localtime(&when); -#endif - return (((tm->tm_year + 1900) * 10000) + - ((tm->tm_mon + 1) * 100) + tm->tm_mday); + struct tm t, *tm = localtime_r(&when, &t); + if (tm == NULL) { + return (0); + } + return (((tm->tm_year + 1900) * 10000) + ((tm->tm_mon + 1) * 100) + + tm->tm_mday); } uint32_t diff --git a/lib/isc/win32/include/isc/time.h b/lib/isc/win32/include/isc/time.h index 486f527c60..8494736fea 100644 --- a/lib/isc/win32/include/isc/time.h +++ b/lib/isc/win32/include/isc/time.h @@ -13,6 +13,7 @@ #ifndef ISC_TIME_H #define ISC_TIME_H 1 +#include #include #include #include @@ -20,6 +21,30 @@ #include #include +/*** + *** POSIX Shims + ***/ + +inline struct tm * +gmtime_r(const time_t *clock, struct tm *result) { + errno_t ret = gmtime_s(result, clock); + if (ret != 0) { + errno = ret; + return (NULL); + } + return (result); +} + +inline struct tm * +localtime_r(const time_t *clock, struct tm *result) { + errno_t ret = localtime_s(result, clock); + if (ret != 0) { + errno = ret; + return (NULL); + } + return (result); +} + /*** *** Intervals ***/