diff --git a/CHANGES b/CHANGES index d0b55f649c..2fdf06268c 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +4307. [bug] "dig +subnet" and "mdig +subnet" could send + incorrectly-formatted Client Subnet options + if the prefix length was not divisble by 8. + Also fixed a memory leak in "mdig". [RT #45178] + 4306. [maint] Added a PKCS#11 openssl patch supporting version 1.0.2f [RT #38312] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 9477477738..8c43666373 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -1104,7 +1104,7 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { } else if (netmask != 0xffffffff) { int i; - for (i = 0; i < 3; i++) { + for (i = 0; i < 3 && strlen(buf) < sizeof(buf) - 2; i++) { strlcat(buf, ".0", sizeof(buf)); if (inet_pton(AF_INET, buf, &in4) == 1) { parsed = ISC_TRUE; @@ -2504,23 +2504,18 @@ setup_lookup(dig_lookup_t *lookup) { } if (lookup->ecs_addr != NULL) { - isc_uint32_t prefixlen; + isc_uint8_t addr[16], family; + isc_uint32_t plen; struct sockaddr *sa; struct sockaddr_in *sin; struct sockaddr_in6 *sin6; - const isc_uint8_t *addr; size_t addrl; - isc_uint8_t mask; sa = &lookup->ecs_addr->type.sa; - prefixlen = lookup->ecs_addr->length; + plen = lookup->ecs_addr->length; /* Round up prefix len to a multiple of 8 */ - addrl = (prefixlen + 7) / 8; - if (prefixlen % 8 == 0) - mask = 0xff; - else - mask = 0xffU << (8 - (prefixlen % 8)); + addrl = (plen + 7) / 8; INSIST(i < DNS_EDNSOPTIONS); opts[i].code = DNS_OPT_CLIENT_SUBNET; @@ -2528,41 +2523,33 @@ setup_lookup(dig_lookup_t *lookup) { check_result(result, "isc_buffer_allocate"); isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf)); if (sa->sa_family == AF_INET) { + family = 1; sin = (struct sockaddr_in *) sa; - addr = (isc_uint8_t *) &sin->sin_addr; - /* family */ - isc_buffer_putuint16(&b, 1); - /* source prefix-length */ - isc_buffer_putuint8(&b, prefixlen); - /* scope prefix-length */ - isc_buffer_putuint8(&b, 0); - /* address */ - if (addrl > 0) { - isc_buffer_putmem(&b, addr, - (unsigned)addrl - 1); - isc_buffer_putuint8(&b, - (addr[addrl - 1] & - mask)); - } + memcpy(addr, &sin->sin_addr, 4); + if ((plen % 8) != 0) + addr[addrl-1] &= + ~0 << (8 - (plen % 8)); } else { + family = 2; sin6 = (struct sockaddr_in6 *) sa; - addr = (isc_uint8_t *) &sin6->sin6_addr; - /* family */ - isc_buffer_putuint16(&b, 2); - /* source prefix-length */ - isc_buffer_putuint8(&b, prefixlen); - /* scope prefix-length */ - isc_buffer_putuint8(&b, 0); - /* address */ - if (addrl > 0) { - isc_buffer_putmem(&b, addr, - (unsigned)addrl - 1); - isc_buffer_putuint8(&b, - (addr[addrl - 1] & - mask)); - } + memcpy(addr, &sin6->sin6_addr, 16); } + /* Mask off last address byte */ + if (addrl > 0 && (plen % 8) != 0) + addr[addrl - 1] &= ~0 << (8 - (plen % 8)); + + /* family */ + isc_buffer_putuint16(&b, family); + /* source prefix-length */ + isc_buffer_putuint8(&b, plen); + /* scope prefix-length */ + isc_buffer_putuint8(&b, 0); + /* address */ + if (addrl > 0) + isc_buffer_putmem(&b, addr, + (unsigned)addrl); + opts[i].value = (isc_uint8_t *) ecsbuf; i++; } diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh index 0cc87a245f..ef936118c2 100644 --- a/bin/tests/system/digdelv/tests.sh +++ b/bin/tests/system/digdelv/tests.sh @@ -274,6 +274,17 @@ if [ -x ${DIG} ] ; then grep "CLIENT-SUBNET: ::/0/0" < dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo "I:failed"; fi status=`expr $status + $ret` + + n=`expr $n + 1` + echo "I:checking dig +subnet with prefix lengths between byte boundaries ($n)" + ret=0 + for p in 9 10 11 12 13 14 15; do + $DIG $DIGOPTS +tcp @10.53.0.2 +subnet=10.53/$p A a.example > dig.out.test.$p.$n 2>&1 || ret=1 + grep "FORMERR" < dig.out.test.$p.$n > /dev/null && ret=1 + grep "CLIENT-SUBNET.*/$p/0" < dig.out.test.$p.$n > /dev/null || ret=1 + done + if [ $ret != 0 ]; then echo "I:failed"; fi + status=`expr $status + $ret` n=`expr $n + 1` echo "I:checking dig +sp works as an abbriviated form of split ($n)" diff --git a/bin/tools/mdig.c b/bin/tools/mdig.c index 38c3bc61cf..341891de32 100644 --- a/bin/tools/mdig.c +++ b/bin/tools/mdig.c @@ -584,24 +584,19 @@ sendquery(struct query *query, isc_task_t *task) } if (query->ecs_addr != NULL) { - isc_uint32_t prefixlen; + isc_uint8_t addr[16], family; + isc_uint32_t plen; struct sockaddr *sa; struct sockaddr_in *sin; struct sockaddr_in6 *sin6; - const isc_uint8_t *addr; size_t addrl; - isc_uint8_t mask; isc_buffer_t b; sa = &query->ecs_addr->type.sa; - prefixlen = query->ecs_addr->length; + plen = query->ecs_addr->length; /* Round up prefix len to a multiple of 8 */ - addrl = (prefixlen + 7) / 8; - if (prefixlen % 8 == 0) - mask = 0xff; - else - mask = 0xffU << (8 - (prefixlen % 8)); + addrl = (plen + 7) / 8; INSIST(i < DNS_EDNSOPTIONS); opts[i].code = DNS_OPT_CLIENT_SUBNET; @@ -609,41 +604,33 @@ sendquery(struct query *query, isc_task_t *task) CHECK("isc_buffer_allocate", result); isc_buffer_init(&b, ecsbuf, sizeof(ecsbuf)); if (sa->sa_family == AF_INET) { + family = 1; sin = (struct sockaddr_in *) sa; - addr = (isc_uint8_t *) &sin->sin_addr; - /* family */ - isc_buffer_putuint16(&b, 1); - /* source prefix-length */ - isc_buffer_putuint8(&b, prefixlen); - /* scope prefix-length */ - isc_buffer_putuint8(&b, 0); - /* address */ - if (addrl > 0) { - isc_buffer_putmem(&b, addr, - (unsigned)addrl - 1); - isc_buffer_putuint8(&b, - (addr[addrl - 1] & - mask)); - } + memcpy(addr, &sin->sin_addr, 4); + if ((plen % 8) != 0) + addr[addrl-1] &= + ~0 << (8 - (plen % 8)); } else { + family = 2; sin6 = (struct sockaddr_in6 *) sa; - addr = (isc_uint8_t *) &sin6->sin6_addr; - /* family */ - isc_buffer_putuint16(&b, 2); - /* source prefix-length */ - isc_buffer_putuint8(&b, prefixlen); - /* scope prefix-length */ - isc_buffer_putuint8(&b, 0); - /* address */ - if (addrl > 0) { - isc_buffer_putmem(&b, addr, - (unsigned)addrl - 1); - isc_buffer_putuint8(&b, - (addr[addrl - 1] & - mask)); - } + memcpy(addr, &sin6->sin6_addr, 16); } + /* Mask off last address byte */ + if (addrl > 0 && (plen % 8) != 0) + addr[addrl - 1] &= ~0 << (8 - (plen % 8)); + + /* family */ + isc_buffer_putuint16(&b, family); + /* source prefix-length */ + isc_buffer_putuint8(&b, plen); + /* scope prefix-length */ + isc_buffer_putuint8(&b, 0); + /* address */ + if (addrl > 0) + isc_buffer_putmem(&b, addr, + (unsigned)addrl); + opts[i].value = (isc_uint8_t *) ecsbuf; i++; } @@ -913,6 +900,7 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { slash = strchr(buf, '/'); if (slash != NULL) { + *slash = '\0'; result = isc_parse_uint32(&netmask, slash + 1, 10); if (result != ISC_R_SUCCESS) { fatal("invalid prefix length in '%s': %s\n", @@ -938,7 +926,7 @@ parse_netprefix(isc_sockaddr_t **sap, const char *value) { } else if (netmask != 0xffffffff) { int i; - for (i = 0; i < 3; i++) { + for (i = 0; i < 3 && strlen(buf) < sizeof(buf) - 2; i++) { strlcat(buf, ".0", sizeof(buf)); if (inet_pton(AF_INET, buf, &in4) == 1) { parsed = ISC_TRUE; @@ -1838,8 +1826,7 @@ parse_args(isc_boolean_t is_batchfile, int argc, char **argv) /*% Main processing routine for mdig */ int -main(int argc, char *argv[]) -{ +main(int argc, char *argv[]) { struct query *query; isc_sockaddr_t bind_any; isc_log_t *lctx; @@ -1867,6 +1854,7 @@ main(int argc, char *argv[]) fatal("could not find either IPv4 or IPv6"); mctx = NULL; +isc_mem_debugging = ISC_MEM_DEBUGRECORD; RUNCHECK(isc_mem_create(0, 0, &mctx)); lctx = NULL; @@ -1947,12 +1935,17 @@ main(int argc, char *argv[]) while (query != NULL) { struct query *next = ISC_LIST_NEXT(query, link); - if (query->ecs_addr != NULL) + if (query->ecs_addr != NULL) { isc_mem_free(mctx, query->ecs_addr); + query->ecs_addr = NULL; + } isc_mem_free(mctx, query); query = next; } + if (default_query.ecs_addr != NULL) + isc_mem_free(mctx, default_query.ecs_addr); + dns_view_detach(&view); dns_requestmgr_shutdown(requestmgr);