From f9d24b1b8548363b73a086de9d7038ac2517cd12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 8 Apr 2026 12:53:16 +0200 Subject: [PATCH] Reject oversized RRsets at slab/vec construction makeslab(), makevec(), dns_rdatavec_merge() and dns_rdatavec_subtract() summed per-record storage into an unsigned int with no upper-bound check. An RRset whose total encoded size exceeds DNS_RDATA_MAXLENGTH cannot fit in a DNS message and is unservable; building its in-memory representation only burns memory on data that will fail at response time, and at the upper bound the running sum could in theory wrap. Cap the running total at DNS_RDATA_MAXLENGTH and return ISC_R_NOSPACE when exceeded. Update the qpdb cache memory-purge test to use a record size that fits within the new limit. Assisted-by: Claude:claude-opus-4-7 --- lib/dns/rdataslab.c | 14 ++++++++++--- lib/dns/rdatavec.c | 46 +++++++++++++++++++++++++++++++++---------- tests/dns/qpdb_test.c | 3 ++- 3 files changed, 49 insertions(+), 14 deletions(-) diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c index ef38df0be3..6bbb0cd971 100644 --- a/lib/dns/rdataslab.c +++ b/lib/dns/rdataslab.c @@ -130,7 +130,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, dns_rdata_t *rdata = NULL; unsigned char *rawbuf = NULL; unsigned int headerlen = sizeof(dns_slabheader_t); - unsigned int buflen = headerlen; + uint32_t buflen = headerlen; isc_result_t result; unsigned int nitems; unsigned int nalloc; @@ -231,20 +231,24 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, rdata[i - 1].data = &removed; nitems--; } else { - buflen += (2 + rdata[i - 1].length); + buflen += 2 + rdata[i - 1].length; /* * Provide space to store the per RR meta data. */ if (rdataset->type == dns_rdatatype_rrsig) { buflen++; } + if (buflen - headerlen > DNS_RDATA_MAXLENGTH) { + result = ISC_R_NOSPACE; + goto free_rdatas; + } } } /* * Don't forget the last item! */ - buflen += (2 + rdata[i - 1].length); + buflen += 2 + rdata[i - 1].length; /* * Provide space to store the per RR meta data. @@ -252,6 +256,10 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, if (rdataset->type == dns_rdatatype_rrsig) { buflen++; } + if (buflen - headerlen > DNS_RDATA_MAXLENGTH) { + result = ISC_R_NOSPACE; + goto free_rdatas; + } /* * Ensure that singleton types are actually singletons. diff --git a/lib/dns/rdatavec.c b/lib/dns/rdatavec.c index 80a3446802..b05cc285ba 100644 --- a/lib/dns/rdatavec.c +++ b/lib/dns/rdatavec.c @@ -155,7 +155,7 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, dns_rdata_t *rdata = NULL; unsigned char *rawbuf = NULL; unsigned int headerlen = sizeof(dns_vecheader_t); - unsigned int buflen = headerlen + 2; + uint32_t buflen = headerlen + 2; isc_result_t result; unsigned int nitems; unsigned int nalloc; @@ -256,20 +256,24 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, rdata[i - 1].data = &removed; nitems--; } else { - buflen += (2 + rdata[i - 1].length); + buflen += 2 + rdata[i - 1].length; /* * Provide space to store the per RR meta data. */ if (rdataset->type == dns_rdatatype_rrsig) { buflen++; } + if (buflen - headerlen - 2 > DNS_RDATA_MAXLENGTH) { + result = ISC_R_NOSPACE; + goto free_rdatas; + } } } /* * Don't forget the last item! */ - buflen += (2 + rdata[i - 1].length); + buflen += 2 + rdata[i - 1].length; /* * Provide space to store the per RR meta data. @@ -277,6 +281,10 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region, if (rdataset->type == dns_rdatatype_rrsig) { buflen++; } + if (buflen - headerlen - 2 > DNS_RDATA_MAXLENGTH) { + result = ISC_R_NOSPACE; + goto free_rdatas; + } /* * Ensure that singleton types are actually singletons. @@ -463,7 +471,8 @@ dns_rdatavec_merge(dns_vecheader_t *oheader, dns_vecheader_t *nheader, uint32_t maxrrperset, dns_vecheader_t **theaderp) { isc_result_t result = ISC_R_SUCCESS; unsigned char *ocurrent = NULL, *ncurrent = NULL, *tcurrent = NULL; - unsigned int ocount, ncount, tlength, tcount = 0; + unsigned int ocount, ncount, tcount = 0; + uint32_t tlength; vecinfo_t *oinfo = NULL, *ninfo = NULL; size_t o = 0, n = 0; @@ -488,23 +497,31 @@ dns_rdatavec_merge(dns_vecheader_t *oheader, dns_vecheader_t *nheader, */ tlength = header_size(oheader) + 2; + /* + * Allocate both info arrays up front so the cleanup path is + * always safe to call regardless of where we exit. + */ + oinfo = isc_mem_cget(mctx, ocount, sizeof(struct vecinfo)); + ninfo = isc_mem_cget(mctx, ncount, sizeof(struct vecinfo)); + /* * Gather the rdatas in the old vec and add their lengths to * the larget length. */ - oinfo = isc_mem_cget(mctx, ocount, sizeof(struct vecinfo)); for (size_t i = 0; i < ocount; i++) { oinfo[i].pos = ocurrent; dns_rdata_init(&oinfo[i].rdata); rdata_from_vecitem(&ocurrent, rdclass, type, &oinfo[i].rdata); - tlength += ocurrent - oinfo[i].pos; + tlength += (uint32_t)(ocurrent - oinfo[i].pos); + if (tlength - header_size(oheader) - 2 > DNS_RDATA_MAXLENGTH) { + CLEANUP(ISC_R_NOSPACE); + } } /* * Then add the length of rdatas in the new vec that aren't * duplicated in the old vec. */ - ninfo = isc_mem_cget(mctx, ncount, sizeof(struct vecinfo)); for (size_t i = 0; i < ncount; i++) { ninfo[i].pos = ncurrent; dns_rdata_init(&ninfo[i].rdata); @@ -542,7 +559,10 @@ dns_rdatavec_merge(dns_vecheader_t *oheader, dns_vecheader_t *nheader, * We will be copying this item to the target, so * add its length to tlength and increment tcount. */ - tlength += ncurrent - ninfo[i].pos; + tlength += (uint32_t)(ncurrent - ninfo[i].pos); + if (tlength - header_size(oheader) - 2 > DNS_RDATA_MAXLENGTH) { + CLEANUP(ISC_R_NOSPACE); + } tcount++; } @@ -659,7 +679,8 @@ dns_rdatavec_subtract(dns_vecheader_t *oheader, dns_vecheader_t *sheader, isc_result_t result = ISC_R_SUCCESS; unsigned char *ocurrent = NULL, *scurrent = NULL; unsigned char *tstart = NULL, *tcurrent = NULL; - unsigned int ocount, scount, tlength; + unsigned int ocount, scount; + uint32_t tlength; unsigned int tcount = 0, rcount = 0; vecinfo_t *oinfo = NULL, *sinfo = NULL; @@ -721,7 +742,12 @@ dns_rdatavec_subtract(dns_vecheader_t *oheader, dns_vecheader_t *sheader, * so copy it to the target. Add its length to * tlength and increment tcount. */ - tlength += ocurrent - oinfo[i].pos; + tlength += (uint32_t)(ocurrent - oinfo[i].pos); + if (tlength - header_size(oheader) - 2 > + DNS_RDATA_MAXLENGTH) + { + CLEANUP(ISC_R_NOSPACE); + } tcount++; } } diff --git a/tests/dns/qpdb_test.c b/tests/dns/qpdb_test.c index 287e80de0c..de3ea1245b 100644 --- a/tests/dns/qpdb_test.c +++ b/tests/dns/qpdb_test.c @@ -157,7 +157,8 @@ ISC_LOOP_TEST_IMPL(overmempurge_bigrdata) { * cache size doesn't reach the "max". */ while (i-- > 0) { - overmempurge_addrdataset(db, now, i, 50054, 65535, false); + overmempurge_addrdataset(db, now, i, 50054, + DNS_RDATA_MAXLENGTH - 2, false); cleanup_all_deadnodes(db); if (verbose) { print_message("# inuse: %zd max: %zd\n",