From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Fri, 15 Jun 2018 09:05:40 +0000 (+0200)
Subject: cache: avoid potential out-of-bounds with NSEC3 params
X-Git-Tag: v2.4.0~19^2~7
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=703c21cb8b3e02c8d253545205379474efdd7085;p=thirdparty%2Fknot-resolver.git

cache: avoid potential out-of-bounds with NSEC3 params

It's possible the parser wouldn't let such RR through,
and it's most likely validator shouldn't let them through.
Even so, I feel better to check anyway.
---

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 0fa6f5770..1344ebe71 100644
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -473,7 +473,10 @@ static ssize_t stash_rrset(struct kr_cache *cache, const struct kr_query *qry,
 
 		assert(rr->type == KNOT_RRTYPE_NSEC3);
 		const knot_rdata_t *np_data = knot_rdata_data(rr->rrs.data);
+		const int rdlen = knot_rdata_rdlen(rr->rrs.data);
+		if (rdlen <= 4) return kr_error(EILSEQ); /*< data from outside; less trust */
 		const int np_dlen = nsec_p_rdlen(np_data);
+		if (np_dlen > rdlen) return kr_error(EILSEQ);
 		key = key_NSEC3(k, encloser, nsec_p_mkHash(np_data));
 		if (npp && !*npp) {
 			*npp = mm_alloc(&qry->request->pool, np_dlen);
@@ -606,6 +609,7 @@ static int stash_rrarray_entry(ranked_rr_array_t *arr, int arr_i,
 	ssize_t written = stash_rrset(cache, qry, rr, rr_sigs, qry->timestamp.tv_sec,
 					entry->rank, nsec_pmap, has_optout);
 	if (written < 0) {
+		kr_log_error("[%5hu][cach] stash failed, ret = %d\n", qry->id, ret);
 		return (int) written;
 	}
 
diff --git a/lib/cache/impl.h b/lib/cache/impl.h
index 74271315e..d72f74faa 100644
--- a/lib/cache/impl.h
+++ b/lib/cache/impl.h
@@ -80,7 +80,7 @@ static inline struct entry_h * entry_h_consistent_NSEC(knot_db_val_t data)
 
 static inline int nsec_p_rdlen(const uint8_t *rdata)
 {
-	//TODO: the zero case? // FIXME security: overflow potential
+	//TODO: the zero case?
 	return rdata ? 5 + rdata[4] : 0; /* rfc5155 4.2 and 3.2. */
 }
 static const int NSEC_P_MAXLEN = sizeof(uint32_t) + 5 + 255; // TODO: remove??