From b16d99bac1d100735224ab3eaa84632537ff21b5 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Jun 2014 09:17:15 +1000 Subject: [PATCH] 3872. [bug] Address issues found by static analysis. [RT #36209] --- CHANGES | 2 ++ bin/named/update.c | 5 ++-- bin/tests/dst/gsstest.c | 24 ++++++++++--------- bin/tools/genrandom.c | 4 +++- .../dlz/modules/mysqldyn/dlz_mysqldyn_mod.c | 2 +- lib/dns/nsec3.c | 1 + lib/dns/rbt.c | 5 +++- lib/isc/include/isc/iterated_hash.h | 2 +- 8 files changed, 27 insertions(+), 18 deletions(-) diff --git a/CHANGES b/CHANGES index e47f816285..9a52b06d63 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +3872. [bug] Address issues found by static analysis. [RT #36209] + 3871. [bug] Don't publish an activated key automatically before its publish time. [RT #35063] diff --git a/bin/named/update.c b/bin/named/update.c index 0713fde579..021b70562e 100644 --- a/bin/named/update.c +++ b/bin/named/update.c @@ -3096,10 +3096,9 @@ update_action(isc_task_t *task, isc_event_t *event) { #define ALLOW_SECURE_TO_INSECURE(zone) \ ((dns_zone_getoptions(zone) & DNS_ZONEOPT_SECURETOINSECURE) != 0) + CHECK(rrset_exists(db, oldver, zonename, dns_rdatatype_dnskey, + 0, &had_dnskey)); if (!ALLOW_SECURE_TO_INSECURE(zone)) { - CHECK(rrset_exists(db, oldver, zonename, - dns_rdatatype_dnskey, 0, - &had_dnskey)); if (had_dnskey && !has_dnskey) { update_log(client, zone, LOGLEVEL_PROTOCOL, "update rejected: all DNSKEY " diff --git a/bin/tests/dst/gsstest.c b/bin/tests/dst/gsstest.c index cb08d2bdcb..65a5837092 100644 --- a/bin/tests/dst/gsstest.c +++ b/bin/tests/dst/gsstest.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -71,9 +72,6 @@ struct dst_context { } \ } -static char contextname[512]; -static char gssid[512]; -static char serveraddress[512]; static dns_fixedname_t servername, gssname; static isc_mem_t *mctx; @@ -106,7 +104,7 @@ console(isc_task_t *task, isc_event_t *event) for (;;) { printf("\nCommand => "); - c = scanf("%s", buf); + c = scanf("%31s", buf); if (c == EOF || strcmp(buf, "quit") == 0) { isc_app_shutdown(); @@ -209,7 +207,7 @@ sendquery(isc_task_t *task, isc_event_t *event) isc_event_free(&event); printf("Query => "); - c = scanf("%s", host); + c = scanf("%255s", host); if (c == EOF) return; @@ -350,6 +348,8 @@ initctx2(isc_task_t *task, isc_event_t *event) { static void initctx1(isc_task_t *task, isc_event_t *event) { + char gssid[512]; + char contextname[512]; isc_result_t result; isc_buffer_t buf; dns_message_t *query; @@ -359,11 +359,12 @@ initctx1(isc_task_t *task, isc_event_t *event) { isc_event_free(&event); printf("Initctx - GSS name => "); - c = scanf("%s", gssid); + c = scanf("%511s", gssid); if (c == EOF) return; - sprintf(contextname, "gsstest.context.%d.", (int)time(NULL)); + snprintf(contextname, sizeof(contextname), + "gsstest.context.%d.", (int)time(NULL)); printf("Initctx - context name we're using: %s\n", contextname); @@ -417,12 +418,13 @@ initctx1(isc_task_t *task, isc_event_t *event) { static void setup(void) { - struct in_addr inaddr; - int c; - for (;;) { + char serveraddress[512]; + struct in_addr inaddr; + int c; + printf("Server IP => "); - c = scanf("%s", serveraddress); + c = scanf("%511s", serveraddress); if (c == EOF || strcmp(serveraddress, "quit") == 0) { isc_app_shutdown(); diff --git a/bin/tools/genrandom.c b/bin/tools/genrandom.c index 0d7eb726d6..a48af83b8f 100644 --- a/bin/tools/genrandom.c +++ b/bin/tools/genrandom.c @@ -120,7 +120,9 @@ main(int argc, char **argv) { return (0); } - len = strlen(argv[isc_commandline_index]) + 2; + len = strlen(argv[isc_commandline_index]); + INSIST((len + 2) > len); + len += 2; name = (char *) malloc(len); if (name == NULL) { perror("malloc"); diff --git a/contrib/dlz/modules/mysqldyn/dlz_mysqldyn_mod.c b/contrib/dlz/modules/mysqldyn/dlz_mysqldyn_mod.c index c0ee9f3d04..3f360c3964 100644 --- a/contrib/dlz/modules/mysqldyn/dlz_mysqldyn_mod.c +++ b/contrib/dlz/modules/mysqldyn/dlz_mysqldyn_mod.c @@ -1620,7 +1620,7 @@ dlz_addrdataset(const char *name, const char *rdatastr, * SOA: zone admin serial refresh retry expire min */ char sn[32]; - sscanf(record->data, "%*s %*s %s %*s %*s %*s %*s", sn); + sscanf(record->data, "%*s %*s %31s %*s %*s %*s %*s", sn); query = build_query(state, txn->dbi, U_SERIAL, sn, txn->zone_id); if (query == NULL) { diff --git a/lib/dns/nsec3.c b/lib/dns/nsec3.c index e66cc5a56f..62685f30d6 100644 --- a/lib/dns/nsec3.c +++ b/lib/dns/nsec3.c @@ -567,6 +567,7 @@ dns_nsec3_addnsec3(dns_db_t *db, dns_dbversion_t *version, CHECK(dns_nsec3_hashname(&fixed, nexthash, &next_length, name, origin, hash, iterations, salt, salt_length)); + INSIST(next_length <= sizeof(nexthash)); /* * Create the node if it doesn't exist and hold diff --git a/lib/dns/rbt.c b/lib/dns/rbt.c index 32ddbde999..7a440274aa 100644 --- a/lib/dns/rbt.c +++ b/lib/dns/rbt.c @@ -263,7 +263,10 @@ getdata(dns_rbtnode_t *node, file_header_t *header) { * of memory concerns, when chains were first implemented). */ #define ADD_LEVEL(chain, node) \ - (chain)->levels[(chain)->level_count++] = (node) + do { \ + INSIST((chain)->level_count < DNS_RBT_LEVELBLOCK); \ + (chain)->levels[(chain)->level_count++] = (node); \ + } while (0) /*% * The following macros directly access normally private name variables. diff --git a/lib/isc/include/isc/iterated_hash.h b/lib/isc/include/isc/iterated_hash.h index a8173f0ab2..ba9fa93b96 100644 --- a/lib/isc/include/isc/iterated_hash.h +++ b/lib/isc/include/isc/iterated_hash.h @@ -23,7 +23,7 @@ #include /* - * The maximal hash length that can be encoded it a name + * The maximal hash length that can be encoded in a name * using base32hex. floor(255/8)*5 */ #define NSEC3_MAX_HASH_LENGTH 155