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

diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index ef38df0be3857cd7b2e0e1512c26fb234006a1f1..6bbb0cd9715bdccdef3d622e1d379e508a376d14 100644 (file)
--- 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 80a3446802cc3f979e02e041a09d56ea855e9fa2..b05cc285ba17797ab40625361ace61bb6f7c4318 100644 (file)
--- 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 287e80de0c5aa9e4aa3d3de9ebcb973b77a154dd..de3ea1245bb9dd5466e2da001dc2924c8cc5d494 100644 (file)
--- 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",