From b292230ab8dd33480dabad2b3615dcce5dd70c35 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 29 Apr 2015 03:16:50 +1000 Subject: [PATCH] 4110. [bug] Address memory leaks / null pointer dereferences on out of memory. [RT #39310] --- CHANGES | 3 +++ bin/dig/dighost.c | 4 ++++ bin/named/config.c | 16 ++++++++++------ bin/tests/db_test.c | 4 +++- bin/tests/sock_test.c | 23 ++++++++++++++++------- bin/tools/mdig.c | 2 ++ lib/dns/include/dns/name.h | 1 + lib/dns/name.c | 2 ++ lib/dns/tests/rbt_test.c | 3 +++ lib/isc/include/isc/mem.h | 14 ++++++++++++++ lib/isc/tests/random_test.c | 3 ++- lib/isccfg/parser.c | 3 ++- 12 files changed, 62 insertions(+), 16 deletions(-) diff --git a/CHANGES b/CHANGES index 7633bded32..f3d379eb2b 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +4110. [bug] Address memory leaks / null pointer dereferences + on out of memory. [RT #39310] + 4109. [port] linux: support reading the local port range from net.ipv4.ip_local_port_range. [RT # 39379] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 35a86b3223..c2bb8c4b28 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -933,6 +933,8 @@ clone_lookup(dig_lookup_t *lookold, isc_boolean_t servers) { if (lookold->ecs_addr != NULL) { size_t len = sizeof(isc_sockaddr_t); looknew->ecs_addr = isc_mem_allocate(mctx, len); + if (looknew->ecs_addr == NULL) + fatal("out of memory"); memmove(looknew->ecs_addr, lookold->ecs_addr, len); } @@ -1086,6 +1088,8 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { } sa = isc_mem_allocate(mctx, sizeof(*sa)); + if (sa == NULL) + fatal("out of memory"); if (inet_pton(AF_INET6, value, &in6) == 1) { isc_sockaddr_fromin6(sa, &in6, 0); parsed = ISC_TRUE; diff --git a/bin/named/config.c b/bin/named/config.c index 6107159c0e..d4a1f21560 100644 --- a/bin/named/config.c +++ b/bin/named/config.c @@ -465,10 +465,6 @@ ns_config_getiplist(const cfg_obj_t *config, const cfg_obj_t *list, } if (dscpsp != NULL) { - dscps = isc_mem_get(mctx, count * sizeof(isc_dscp_t)); - if (dscps == NULL) - return (ISC_R_NOMEMORY); - dscpobj = cfg_tuple_get(list, "dscp"); if (dscpobj != NULL && cfg_obj_isuint32(dscpobj)) { if (cfg_obj_asuint32(dscpobj) > 63) { @@ -479,11 +475,18 @@ ns_config_getiplist(const cfg_obj_t *config, const cfg_obj_t *list, } dscp = (isc_dscp_t)cfg_obj_asuint32(dscpobj); } + + dscps = isc_mem_get(mctx, count * sizeof(isc_dscp_t)); + if (dscps == NULL) + return (ISC_R_NOMEMORY); } addrs = isc_mem_get(mctx, count * sizeof(isc_sockaddr_t)); - if (addrs == NULL) + if (addrs == NULL) { + if (dscps != NULL) + isc_mem_put(mctx, dscps, count * sizeof(isc_dscp_t)); return (ISC_R_NOMEMORY); + } for (element = cfg_list_first(addrlist); element != NULL; @@ -622,7 +625,8 @@ ns_config_getipandkeylist(const cfg_obj_t *config, const cfg_obj_t *list, cfg_obj_log(dscpobj, ns_g_lctx, ISC_LOG_ERROR, "dscp value '%u' is out of range", cfg_obj_asuint32(dscpobj)); - return (ISC_R_RANGE); + result = ISC_R_RANGE; + goto cleanup; } dscp = (isc_dscp_t)cfg_obj_asuint32(dscpobj); } diff --git a/bin/tests/db_test.c b/bin/tests/db_test.c index d3d8b4a543..54ab207545 100644 --- a/bin/tests/db_test.c +++ b/bin/tests/db_test.c @@ -266,8 +266,10 @@ load(const char *filename, const char *origintext, isc_boolean_t cache) { dns_fixedname_init(&forigin); origin = dns_fixedname_name(&forigin); result = dns_name_fromtext(origin, &source, dns_rootname, 0, NULL); - if (result != ISC_R_SUCCESS) + if (result != ISC_R_SUCCESS) { + isc_mem_put(mctx, dbi, sizeof(*dbi)); return (result); + } result = dns_db_create(mctx, dbtype, origin, cache ? dns_dbtype_cache : dns_dbtype_zone, diff --git a/bin/tests/sock_test.c b/bin/tests/sock_test.c index 6ced499f68..009ce3cc36 100644 --- a/bin/tests/sock_test.c +++ b/bin/tests/sock_test.c @@ -61,7 +61,8 @@ my_send(isc_task_t *task, isc_event_t *event) { isc_task_shutdown(task); } - isc_mem_put(mctx, dev->region.base, dev->region.length); + if (dev->region.base != NULL) + isc_mem_put(mctx, dev->region.base, dev->region.length); isc_event_free(&event); } @@ -96,8 +97,8 @@ my_recv(isc_task_t *task, isc_event_t *event) { if (dev->result != ISC_R_SUCCESS) { isc_socket_detach(&sock); - isc_mem_put(mctx, dev->region.base, - dev->region.length); + if (dev->region.base != NULL) + isc_mem_put(mctx, dev->region.base, dev->region.length); isc_event_free(&event); isc_task_shutdown(task); @@ -112,8 +113,11 @@ my_recv(isc_task_t *task, isc_event_t *event) { sprintf(buf, "\r\nReceived: %.*s\r\n\r\n", (int)dev->n, (char *)region.base); region.base = isc_mem_get(mctx, strlen(buf) + 1); - region.length = strlen(buf) + 1; - strcpy((char *)region.base, buf); /* strcpy is safe */ + if (region.base != NULL) { + region.length = strlen(buf) + 1; + strcpy((char *)region.base, buf); /* strcpy is safe */ + } else + region.length = 0; isc_socket_send(sock, ®ion, task, my_send, event->ev_arg); } else { region = dev->region; @@ -143,6 +147,8 @@ my_http_get(isc_task_t *task, isc_event_t *event) { if (dev->result != ISC_R_SUCCESS) { isc_socket_detach(&sock); isc_task_shutdown(task); + if (dev->region.base != NULL) + isc_mem_put(mctx, dev->region.base, dev->region.length); isc_event_free(&event); return; } @@ -179,8 +185,11 @@ my_connect(isc_task_t *task, isc_event_t *event) { strcpy(buf, "GET / HTTP/1.1\r\nHost: www.flame.org\r\n" "Connection: Close\r\n\r\n"); region.base = isc_mem_get(mctx, strlen(buf) + 1); - region.length = strlen(buf) + 1; - strcpy((char *)region.base, buf); /* This strcpy is safe. */ + if (region.base != NULL) { + region.length = strlen(buf) + 1; + strcpy((char *)region.base, buf); /* This strcpy is safe. */ + } else + region.length = 0; isc_socket_send(sock, ®ion, task, my_http_get, event->ev_arg); diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index bdfa578035..9d96e83f2e 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -908,6 +908,8 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { } sa = isc_mem_allocate(mctx, sizeof(*sa)); + if (sa == NULL) + fatal("out of memory"); if (inet_pton(AF_INET6, value, &in6) == 1) { isc_sockaddr_fromin6(sa, &in6, 0); parsed = ISC_TRUE; diff --git a/lib/dns/include/dns/name.h b/lib/dns/include/dns/name.h index 4430d70093..103128d81e 100644 --- a/lib/dns/include/dns/name.h +++ b/lib/dns/include/dns/name.h @@ -1167,6 +1167,7 @@ dns_name_tostring(dns_name_t *source, char **target, isc_mem_t *mctx); * Returns: * *\li ISC_R_SUCCESS + *\li ISC_R_NOMEMORY * *\li Any error that dns_name_totext() can return. */ diff --git a/lib/dns/name.c b/lib/dns/name.c index 09e44abf64..92ddfc860b 100644 --- a/lib/dns/name.c +++ b/lib/dns/name.c @@ -2428,6 +2428,8 @@ dns_name_tostring(dns_name_t *name, char **target, isc_mem_t *mctx) { isc_buffer_usedregion(&buf, ®); p = isc_mem_allocate(mctx, reg.length + 1); + if (p == NULL) + return (ISC_R_NOMEMORY); memmove(p, (char *) reg.base, (int) reg.length); p[reg.length] = '\0'; diff --git a/lib/dns/tests/rbt_test.c b/lib/dns/tests/rbt_test.c index 7b2f42d722..0d5ea117ea 100644 --- a/lib/dns/tests/rbt_test.c +++ b/lib/dns/tests/rbt_test.c @@ -166,6 +166,7 @@ test_context_setup(void) { size_t i; ctx = isc_mem_get(mctx, sizeof(*ctx)); + ATF_REQUIRE(ctx != NULL); ctx->rbt = NULL; result = dns_rbt_create(mctx, delete_data, NULL, &ctx->rbt); @@ -1134,6 +1135,8 @@ insert_nodes(dns_rbt_t *mytree, char **names, char namebuf[34]; n = isc_mem_get(mctx, sizeof(size_t)); + ATF_REQUIRE(n != NULL); + *n = i; /* Unused value */ while (1) { diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 6714389197..ede0327129 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -49,7 +49,14 @@ typedef void (*isc_memfree_t)(void *, void *); /*% * Define ISC_MEM_CHECKOVERRUN=1 to turn on checks for using memory outside * the requested space. This will increase the size of each allocation. + * + * If we are performing a Coverity static analysis then ISC_MEM_CHECKOVERRUN + * can hide bugs that would otherwise discovered so force to zero. */ +#ifdef __COVERITY__ +#undef ISC_MEM_CHECKOVERRUN +#define ISC_MEM_CHECKOVERRUN 0 +#endif #ifndef ISC_MEM_CHECKOVERRUN #define ISC_MEM_CHECKOVERRUN 1 #endif @@ -59,7 +66,14 @@ typedef void (*isc_memfree_t)(void *, void *); * with the byte string '0xbe'. This helps track down uninitialized pointers * and the like. On freeing memory, the space is filled with '0xde' for * the same reasons. + * + * If we are performing a Coverity static analysis then ISC_MEM_FILL + * can hide bugs that would otherwise discovered so force to zero. */ +#ifdef __COVERITY__ +#undef ISC_MEM_FILL +#define ISC_MEM_FILL 0 +#endif #ifndef ISC_MEM_FILL #define ISC_MEM_FILL 1 #endif diff --git a/lib/isc/tests/random_test.c b/lib/isc/tests/random_test.c index ff53850874..c8f70ba35f 100644 --- a/lib/isc/tests/random_test.c +++ b/lib/isc/tests/random_test.c @@ -371,7 +371,7 @@ monobit(isc_mem_t *mctx, isc_uint16_t *values, size_t length) { /* Debug message, not displayed when running via atf-run */ printf("numbits=%u, scount=%d\n", numbits, scount); - s_obs = fabs(scount) / sqrt(numbits); + s_obs = abs(scount) / sqrt(numbits); p_value = erfc(s_obs / sqrt(2.0)); return (p_value); @@ -484,6 +484,7 @@ blockfrequency(isc_mem_t *mctx, isc_uint16_t *values, size_t length) { ATF_REQUIRE(numbits >= (mbits * numblocks)); pi = isc_mem_get(mctx, numblocks * sizeof(double)); + ATF_REQUIRE(pi != NULL); cur_word = 0; for (i = 0; i < numblocks; i++) { diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c index fc2777c9b9..497ab377e4 100644 --- a/lib/isccfg/parser.c +++ b/lib/isccfg/parser.c @@ -1476,10 +1476,11 @@ parse_any_named_map(cfg_parser_t *pctx, cfg_type_t *nametype, const cfg_type_t * CHECK(cfg_parse_obj(pctx, nametype, &idobj)); CHECK(cfg_parse_map(pctx, type, &mapobj)); mapobj->value.map.id = idobj; - idobj = NULL; *ret = mapobj; + return (result); cleanup: CLEANUP_OBJ(idobj); + CLEANUP_OBJ(mapobj); return (result); }