From: Wouter Wijngaards <wouter@nlnetlabs.nl>
Date: Fri, 20 Mar 2015 15:36:25 +0000 (+0000)
Subject: - Fixes to add integer overflow checks on allocation (defense in depth).
X-Git-Tag: release-1.5.4~59
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6feb8fb6a5dfed83711934b3b9e0327c5a380568;p=thirdparty%2Funbound.git

- 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 20a46ae4d..2780397e7 100644
--- 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 eaaa26f7c..63cc8084e 100644
--- 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 0a24cd1bc..54ddcf9f4 100644
--- 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 687477c24..453f2ad50 100644
--- 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 4692744a1..9a24cbcdf 100644
--- 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 5f52dbce1..564dd5c08 100644
--- 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 9df0d95b4..a2d717307 100644
--- 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 4b81beb4c..a23c6d57e 100644
--- 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 c87c666ac..c4e274e9f 100644
--- 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 5d7990a2b..6039aef24 100644
--- 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 5a4d0f471..2072d106b 100644
--- 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 45ac18b11..d24b6971d 100644
--- 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);