diff --git a/CHANGES b/CHANGES index 0f4fc9e183..5f96f3eacf 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +4869. [bug] Address some cases where NULL with zero length could + be passed to memmove which is undefined behaviour and + can lead to bad optimisation. [RT #46888] + 4868. [func] dnssec-keygen can no longer generate HMAC keys. Use tsig-keygen instead. [RT #46404] diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 84c1bea024..4d771bebb2 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -1002,7 +1002,7 @@ catz_process_version(dns_catz_zone_t *zone, dns_rdataset_t *value) { result = ISC_R_BADNUMBER; goto cleanup; } - memcpy(t, rdatastr.data, rdatastr.length); + memmove(t, rdatastr.data, rdatastr.length); t[rdatastr.length] = 0; result = isc_parse_uint32(&tversion, t, 10); if (result != ISC_R_SUCCESS) { @@ -1097,7 +1097,7 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, if (keyname == NULL) return (ISC_R_NOMEMORY); dns_name_init(keyname, 0); - memcpy(keycbuf, rdatastr.data, rdatastr.length); + memmove(keycbuf, rdatastr.data, rdatastr.length); keycbuf[rdatastr.length] = 0; result = dns_name_fromstring(keyname, keycbuf, 0, mctx); if (result != ISC_R_SUCCESS) { @@ -1126,8 +1126,8 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, if (value->type == dns_rdatatype_txt) ipkl->keys[i] = keyname; else /* A/AAAA */ - memcpy(&ipkl->addrs[i], &sockaddr, - sizeof(isc_sockaddr_t)); + memmove(&ipkl->addrs[i], &sockaddr, + sizeof(isc_sockaddr_t)); } else { result = dns_ipkeylist_resize(mctx, ipkl, i+1); @@ -1158,8 +1158,8 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl, if (value->type == dns_rdatatype_txt) ipkl->keys[i] = keyname; else /* A/AAAA */ - memcpy(&ipkl->addrs[i], &sockaddr, - sizeof(isc_sockaddr_t)); + memmove(&ipkl->addrs[i], &sockaddr, + sizeof(isc_sockaddr_t)); ipkl->count++; } return (ISC_R_SUCCESS); @@ -1251,7 +1251,8 @@ catz_process_apl(dns_catz_zone_t *zone, isc_buffer_t **aclbp, result = dns_rdata_apl_current(&rdata_apl, &apl_ent); RUNTIME_CHECK(result == ISC_R_SUCCESS); memset(buf, 0, sizeof(buf)); - memcpy(buf, apl_ent.data, apl_ent.length); + if (apl_ent.data != NULL && apl_ent.length > 0) + memmove(buf, apl_ent.data, apl_ent.length); if (apl_ent.family == 1) isc_netaddr_fromin(&addr, (struct in_addr*) buf); else if (apl_ent.family == 2) diff --git a/lib/dns/diff.c b/lib/dns/diff.c index 75fdd8a34c..0c3d93169c 100644 --- a/lib/dns/diff.c +++ b/lib/dns/diff.c @@ -80,11 +80,16 @@ dns_difftuple_create(isc_mem_t *mctx, t->ttl = ttl; - memmove(datap, rdata->data, rdata->length); dns_rdata_init(&t->rdata); dns_rdata_clone(rdata, &t->rdata); - t->rdata.data = datap; - datap += rdata->length; + if (rdata->data != NULL) { + memmove(datap, rdata->data, rdata->length); + t->rdata.data = datap; + datap += rdata->length; + } else { + t->rdata.data = NULL; + INSIST(rdata->length == 0); + } ISC_LINK_INIT(&t->rdata, link); ISC_LINK_INIT(t, link); diff --git a/lib/dns/ipkeylist.c b/lib/dns/ipkeylist.c index dadcfd6df7..be6d8a7b88 100644 --- a/lib/dns/ipkeylist.c +++ b/lib/dns/ipkeylist.c @@ -187,34 +187,42 @@ dns_ipkeylist_resize(isc_mem_t *mctx, dns_ipkeylist_t *ipkl, unsigned int n) { if (labels == NULL) goto nomemory; - memmove(addrs, ipkl->addrs, ipkl->allocated * sizeof(isc_sockaddr_t)); - if (ipkl->addrs != NULL) + if (ipkl->addrs != NULL) { + memmove(addrs, ipkl->addrs, + ipkl->allocated * sizeof(isc_sockaddr_t)); isc_mem_put(mctx, ipkl->addrs, ipkl->allocated * sizeof(isc_sockaddr_t)); + } ipkl->addrs = addrs; memset(&ipkl->addrs[ipkl->allocated], 0, (n - ipkl->allocated) * sizeof(isc_sockaddr_t)); - memmove(dscps, ipkl->dscps, ipkl->allocated * sizeof(isc_dscp_t)); - if (ipkl->dscps != NULL) + if (ipkl->dscps != NULL) { + memmove(dscps, ipkl->dscps, + ipkl->allocated * sizeof(isc_dscp_t)); isc_mem_put(mctx, ipkl->dscps, ipkl->allocated * sizeof(isc_dscp_t)); + } ipkl->dscps = dscps; memset(&ipkl->dscps[ipkl->allocated], 0, (n - ipkl->allocated) * sizeof(isc_dscp_t)); - memmove(keys, ipkl->keys, ipkl->allocated * sizeof(dns_name_t *)); - if (ipkl->keys) + if (ipkl->keys) { + memmove(keys, ipkl->keys, + ipkl->allocated * sizeof(dns_name_t *)); isc_mem_put(mctx, ipkl->keys, ipkl->allocated * sizeof(dns_name_t *)); + } ipkl->keys = keys; memset(&ipkl->keys[ipkl->allocated], 0, (n - ipkl->allocated) * sizeof(dns_name_t *)); - memmove(labels, ipkl->labels, ipkl->allocated * sizeof(dns_name_t *)); - if (ipkl->labels) + if (ipkl->labels != NULL) { + memmove(labels, ipkl->labels, + ipkl->allocated * sizeof(dns_name_t *)); isc_mem_put(mctx, ipkl->labels, ipkl->allocated * sizeof(dns_name_t *)); + } ipkl->labels = labels; memset(&ipkl->labels[ipkl->allocated], 0, (n - ipkl->allocated) * sizeof(dns_name_t *)); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 5b3cb3479f..40f10461ac 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1810,8 +1810,8 @@ fctx_setretryinterval(fetchctx_t *fctx, unsigned int rtt) { /* * Exponential backoff after the first few tries. */ - if (fctx->restarts >= fctx->res->nonbackofftries) - us <<= (fctx->restarts - fctx->res->nonbackofftries - 1); + if (fctx->restarts > fctx->res->nonbackofftries) + us <<= (fctx->restarts - fctx->res->nonbackofftries); /* * Add a fudge factor to the expected rtt based on the current diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c index 9627e7e065..0b40256f86 100644 --- a/lib/dns/tests/dispatch_test.c +++ b/lib/dns/tests/dispatch_test.c @@ -157,11 +157,11 @@ nameserver(isc_task_t *task, isc_event_t *event) { static unsigned char buf1[16]; static unsigned char buf2[16]; - memcpy(buf1, ev->region.base, 12); + memmove(buf1, ev->region.base, 12); memset(buf1 + 12, 0, 4); buf1[2] |= 0x80; /* qr=1 */ - memcpy(buf2, ev->region.base, 12); + memmove(buf2, ev->region.base, 12); memset(buf2 + 12, 1, 4); buf2[2] |= 0x80; /* qr=1 */