- Fixes to add integer overflow checks on allocation (defense in depth).

git-svn-id: file:///svn/unbound/trunk@3372 be551aaa-1e26-0410-a405-d3ace91eadb9

diff --git a/daemon/cachedump.c b/daemon/cachedump.c
index 20a46ae4dffb4e1851708685a81ff4735519f96c..2780397e7f63d6205da3bea13191b5ca1e7d49ca 100644 (file)
--- a/daemon/cachedump.c
+++ b/daemon/cachedump.c
@@ -223,6 +223,8 @@ copy_msg(struct regional* region, struct lruhash_entry* e,
        struct query_info** k, struct reply_info** d)
 {
        struct reply_info* rep = (struct reply_info*)e->data;
+       if(rep->rrset_count > RR_COUNT_MAX)
+               return 0; /* to protect against integer overflow */
        *d = (struct reply_info*)regional_alloc_init(region, e->data,
                sizeof(struct reply_info) + 
                sizeof(struct rrset_ref) * (rep->rrset_count-1) +
@@ -470,6 +472,10 @@ load_rrset(SSL* ssl, sldns_buffer* buf, struct worker* worker)
                log_warn("bad rrset without contents");
                return 0;
        }
+       if(rr_count > RR_COUNT_MAX || rrsig_count > RR_COUNT_MAX) {
+               log_warn("bad rrset with too many rrs");
+               return 0;
+       }
        d->count = (size_t)rr_count;
        d->rrsig_count = (size_t)rrsig_count;
        d->security = (enum sec_status)security;
@@ -649,6 +655,10 @@ load_msg(SSL* ssl, sldns_buffer* buf, struct worker* worker)
        rep.an_numrrsets = (size_t)an;
        rep.ns_numrrsets = (size_t)ns;
        rep.ar_numrrsets = (size_t)ar;
+       if(an > RR_COUNT_MAX || ns > RR_COUNT_MAX || ar > RR_COUNT_MAX) {
+               log_warn("error too many rrsets");
+               return 0; /* protect against integer overflow in alloc */
+       }
        rep.rrset_count = (size_t)an+(size_t)ns+(size_t)ar;
        rep.rrsets = (struct ub_packed_rrset_key**)regional_alloc_zero(
                region, sizeof(struct ub_packed_rrset_key*)*rep.rrset_count);

diff --git a/dns64/dns64.c b/dns64/dns64.c
index eaaa26f7c91035b4a18f62896575eed46a9137b1..63cc8084e35f07f3231a650eeb64bcd9e5b1765d 100644 (file)
--- a/dns64/dns64.c
+++ b/dns64/dns64.c
@@ -590,6 +590,10 @@ dns64_synth_aaaa_data(const struct ub_packed_rrset_key* fk,
         * for the RRs themselves. Each RR has a length, TTL, pointer to wireformat
         * data, 2 bytes of data length, and 16 bytes of IPv6 address.
         */
+       if(fd->count > RR_COUNT_MAX) {
+               *dd_out = NULL;
+               return; /* integer overflow protection in alloc */
+       }
        if (!(dd = *dd_out = regional_alloc(region,
                  sizeof(struct packed_rrset_data)
                  + fd->count * (sizeof(size_t) + sizeof(time_t) +
@@ -713,6 +717,8 @@ dns64_adjust_a(int id, struct module_qstate* super, struct module_qstate* qstate
                if(i<rep->an_numrrsets && fk->rk.type == htons(LDNS_RR_TYPE_A)) {
                        /* also sets dk->entry.hash */
                        dns64_synth_aaaa_data(fk, fd, dk, &dd, super->region, dns64_env);
+                       if(!dd)
+                               return;
                        /* Delete negative AAAA record from cache stored by
                         * the iterator module */
                        rrset_cache_remove(super->env->rrset_cache, dk->rk.dname, 

diff --git a/doc/Changelog b/doc/Changelog
index 0a24cd1bc8171598f30c23f3419570b1f1ca04ea..54ddcf9f437a83eb6083e1500ba67d825ef74373 100644 (file)
--- a/doc/Changelog
+++ b/doc/Changelog
@@ -1,3 +1,6 @@
+20 March 2015: Wouter
+       - Fixed to add integer overflow checks on allocation (defense in depth).
+
 19 March 2015: Wouter
        - Add ip-transparent config option for bind to non-local addresses.
 

diff --git a/iterator/iterator.c b/iterator/iterator.c
index 687477c243d264088c6af6aac66f3eefc34b518b..453f2ad50ee4147bbda2ac061fc2f6bdc81c152c 100644 (file)
--- a/iterator/iterator.c
+++ b/iterator/iterator.c
@@ -308,6 +308,8 @@ iter_prepend(struct iter_qstate* iq, struct dns_msg* msg,
        if(num_an + num_ns == 0)
                return 1;
        verbose(VERB_ALGO, "prepending %d rrsets", (int)num_an + (int)num_ns);
+       if(num_an > RR_COUNT_MAX || num_ns > RR_COUNT_MAX ||
+               msg->rep->rrset_count > RR_COUNT_MAX) return 0; /* overflow */
        sets = regional_alloc(region, (num_an+num_ns+msg->rep->rrset_count) *
                sizeof(struct ub_packed_rrset_key*));
        if(!sets) 
@@ -2549,6 +2551,12 @@ processClassResponse(struct module_qstate* qstate, int id,
                        /* copy appropriate rcode */
                        to->rep->flags = from->rep->flags;
                        /* copy rrsets */
+                       if(from->rep->rrset_count > RR_COUNT_MAX ||
+                               to->rep->rrset_count > RR_COUNT_MAX) {
+                               log_err("malloc failed (too many rrsets) in collect ANY"); 
+                               foriq->state = FINISHED_STATE;
+                               return; /* integer overflow protection */
+                       }
                        dest = regional_alloc(forq->region, sizeof(dest[0])*n);
                        if(!dest) {
                                log_err("malloc failed in collect ANY"); 

diff --git a/services/cache/dns.c b/services/cache/dns.c
index 4692744a15ddf5fd13fd1967b96942e4b843d732..9a24cbcdff6040cf4a0ada115f82bd50c583c7af 100644 (file)
--- a/services/cache/dns.c
+++ b/services/cache/dns.c
@@ -366,6 +366,8 @@ dns_msg_create(uint8_t* qname, size_t qnamelen, uint16_t qtype,
                sizeof(struct reply_info)-sizeof(struct rrset_ref));
        if(!msg->rep)
                return NULL;
+       if(capacity > RR_COUNT_MAX)
+               return NULL; /* integer overflow protection */
        msg->rep->flags = BIT_QR; /* with QR, no AA */
        msg->rep->qdcount = 1;
        msg->rep->rrsets = (struct ub_packed_rrset_key**)
@@ -453,6 +455,8 @@ gen_dns_msg(struct regional* region, struct query_info* q, size_t num)
                sizeof(struct reply_info) - sizeof(struct rrset_ref));
        if(!msg->rep)
                return NULL;
+       if(num > RR_COUNT_MAX)
+               return NULL; /* integer overflow protection */
        msg->rep->rrsets = (struct ub_packed_rrset_key**)
                regional_alloc(region,
                num * sizeof(struct ub_packed_rrset_key*));

diff --git a/services/cache/rrset.c b/services/cache/rrset.c
index 5f52dbce194826ace70658602e913dee4a55a302..564dd5c08d48a91618a73c04ddd293e5ba83d7ce 100644 (file)
--- a/services/cache/rrset.c
+++ b/services/cache/rrset.c
@@ -304,7 +304,7 @@ rrset_array_unlock_touch(struct rrset_cache* r, struct regional* scratch,
 {
        hashvalue_t* h;
        size_t i;
-       if(!(h = (hashvalue_t*)regional_alloc(scratch, 
+       if(count > RR_COUNT_MAX || !(h = (hashvalue_t*)regional_alloc(scratch, 
                sizeof(hashvalue_t)*count)))
                log_warn("rrset LRU: memory allocation failed");
        else    /* store hash values */

diff --git a/smallapp/unbound-anchor.c b/smallapp/unbound-anchor.c
index 9df0d95b417ce6c550a8575ef2ae7c6b1b410bf8..a2d7173071da962bd4a3e9484805ce018cad63c4 100644 (file)
--- a/smallapp/unbound-anchor.c
+++ b/smallapp/unbound-anchor.c
@@ -915,7 +915,10 @@ read_data_chunk(SSL* ssl, size_t len)
 {
        size_t got = 0;
        int r;
-       char* data = malloc(len+1);
+       char* data;
+       if(len >= 0xfffffff0)
+               return NULL; /* to protect against integer overflow in malloc*/
+       data = malloc(len+1);
        if(!data) {
                if(verb) printf("out of memory\n");
                return NULL;

diff --git a/util/alloc.c b/util/alloc.c
index 4b81beb4c4cb50f549bcbb829d52662d2f4cae5a..a23c6d57e8e14f78aae11d212b5139e1fc9cb4ed 100644 (file)
--- a/util/alloc.c
+++ b/util/alloc.c
@@ -367,8 +367,12 @@ void *unbound_stat_malloc(size_t size)
 /** calloc with stats */
 void *unbound_stat_calloc(size_t nmemb, size_t size)
 {
-       size_t s = (nmemb*size==0)?(size_t)1:nmemb*size;
-       void* res = calloc(1, s+16);
+       size_t s;
+       void* res;
+       if(INT_MAX/nmemb < size)
+               return NULL; /* integer overflow check */
+       s = (nmemb*size==0)?(size_t)1:nmemb*size;
+       res = calloc(1, s+16);
        if(!res) return NULL;
        log_info("stat %p=calloc(%u, %u)", res+16, (unsigned)nmemb, (unsigned)size);
        unbound_mem_alloc += s;
@@ -503,8 +507,12 @@ void *unbound_stat_malloc_lite(size_t size, const char* file, int line,
 void *unbound_stat_calloc_lite(size_t nmemb, size_t size, const char* file,
         int line, const char* func)
 {
-       size_t req = nmemb * size;
-       void* res = malloc(req+lite_pad*2+sizeof(size_t));
+       size_t req;
+       void* res;
+       if(INT_MAX/nmemb < size)
+               return NULL; /* integer overflow check */
+       req = nmemb * size;
+       res = malloc(req+lite_pad*2+sizeof(size_t));
        if(!res) return NULL;
        memmove(res, lite_pre, lite_pad);
        memmove(res+lite_pad, &req, sizeof(size_t));

diff --git a/util/data/msgreply.c b/util/data/msgreply.c
index c87c666ac5e69101a71a069f716d6e0f5e3777cc..c4e274e9fbec8399da35c9ad406f87280fd9890a 100644 (file)
--- a/util/data/msgreply.c
+++ b/util/data/msgreply.c
@@ -87,7 +87,7 @@ construct_reply_info_base(struct regional* region, uint16_t flags, size_t qd,
        /* rrset_count-1 because the first ref is part of the struct. */
        size_t s = sizeof(struct reply_info) - sizeof(struct rrset_ref) +
                sizeof(struct ub_packed_rrset_key*) * total;
-       if(total >= 0xffffff) return NULL; /* sanity check on numRRS*/
+       if(total >= RR_COUNT_MAX) return NULL; /* sanity check on numRRS*/
        if(region)
                rep = (struct reply_info*)regional_alloc(region, s);
        else    rep = (struct reply_info*)malloc(s + 
@@ -278,7 +278,11 @@ parse_create_rrset(sldns_buffer* pkt, struct rrset_parse* pset,
        struct packed_rrset_data** data, struct regional* region)
 {
        /* allocate */
-       size_t s = sizeof(struct packed_rrset_data) + 
+       size_t s;
+       if(pset->rr_count > RR_COUNT_MAX || pset->rrsig_count > RR_COUNT_MAX ||
+               pset->size > RR_COUNT_MAX)
+               return 0; /* protect against integer overflow */
+       s = sizeof(struct packed_rrset_data) + 
                (pset->rr_count + pset->rrsig_count) * 
                (sizeof(size_t)+sizeof(uint8_t*)+sizeof(time_t)) + 
                pset->size;

diff --git a/util/data/packed_rrset.h b/util/data/packed_rrset.h
index 5d7990a2b0b1bac8dfeaa59581b2afb904224a87..6039aef242ca5eee5c6f5ff0e19d213bde4e82de 100644 (file)
--- a/util/data/packed_rrset.h
+++ b/util/data/packed_rrset.h
@@ -58,6 +58,12 @@ typedef uint64_t rrset_id_t;
  * from the SOA in the answer section from a direct SOA query or ANY query. */
 #define PACKED_RRSET_SOA_NEG 0x4
 
+/** number of rrs and rrsets for integer overflow protection.  More than
+ * this is not really possible (64K packet has much less RRs and RRsets) in
+ * a message.  And this is small enough that also multiplied there is no
+ * integer overflow. */
+#define RR_COUNT_MAX 0xffffff
+
 /**
  * The identifying information for an RRset.
  */

diff --git a/validator/val_sigcrypt.c b/validator/val_sigcrypt.c
index 5a4d0f471a84f7c7651c1def47b264981a60035e..2072d106bace1059006e705679bd1329e45c787d 100644 (file)
--- a/validator/val_sigcrypt.c
+++ b/validator/val_sigcrypt.c
@@ -1079,6 +1079,8 @@ int rrset_canonical_equal(struct regional* region,
        fd.rr_data = fdata;
        rbtree_init(&sortree1, &canonical_tree_compare);
        rbtree_init(&sortree2, &canonical_tree_compare);
+       if(d1->count > RR_COUNT_MAX || d2->count > RR_COUNT_MAX)
+               return 1; /* protection against integer overflow */
        rrs1 = regional_alloc(region, sizeof(struct canon_rr)*d1->count);
        rrs2 = regional_alloc(region, sizeof(struct canon_rr)*d2->count);
        if(!rrs1 || !rrs2) return 1; /* alloc failure */
@@ -1135,6 +1137,8 @@ rrset_canonical(struct regional* region, sldns_buffer* buf,
                        sizeof(rbtree_t));
                if(!*sortree)
                        return 0;
+               if(d->count > RR_COUNT_MAX)
+                       return 0; /* integer overflow protection */
                rrs = regional_alloc(region, sizeof(struct canon_rr)*d->count);
                if(!rrs) {
                        *sortree = NULL;

diff --git a/validator/validator.c b/validator/validator.c
index 45ac18b11d94a7824e1b225e4e0af1e54e199791..d24b6971dd44895ee2ca0a2367d805e6af60da83 100644 (file)
--- a/validator/validator.c
+++ b/validator/validator.c
@@ -226,6 +226,8 @@ val_new_getmsg(struct module_qstate* qstate, struct val_qstate* vq)
                sizeof(struct reply_info) - sizeof(struct rrset_ref));
        if(!vq->chase_reply)
                return NULL;
+       if(vq->orig_msg->rep->rrset_count > RR_COUNT_MAX)
+               return NULL; /* protect against integer overflow */
        vq->chase_reply->rrsets = regional_alloc_init(qstate->region,
                vq->orig_msg->rep->rrsets, sizeof(struct ub_packed_rrset_key*)
                        * vq->orig_msg->rep->rrset_count);