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
This commit is contained in:
Ondřej Surý 2026-04-08 12:53:16 +02:00 committed by Ondřej Surý
parent 1535b32dab
commit f9d24b1b85
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41
3 changed files with 49 additions and 14 deletions

View file

@ -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.

View file

@ -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++;
}
}

View file

@ -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",