From 7602be276a73a6eb5431c5acd9718e68a55e8b61 Mon Sep 17 00:00:00 2001
From: Mark Andrews <marka@isc.org>
Date: Mon, 29 Feb 2016 07:16:48 +1100
Subject: [PATCH] Part 2 of: 4319.   [security]      Fix resolver assertion
 failure due to improper                         DNAME handling when parsing
 fetch reply messages.                         (CVE-2016-1286) [RT #41753]

(cherry picked from commit 2de89ee9de8c8da9dc153a754b02dcdbb7fe2374)
Signed-off-by: Sona Sarmadi <sona.sarmadi@enea.com>
---
 lib/dns/resolver.c | 192 ++++++++++++++++++++++++++---------------------------
 1 file changed, 93 insertions(+), 99 deletions(-)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 70aba87..41e9df4 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -6074,14 +6074,11 @@ cname_target(dns_rdataset_t *rdataset, dns_name_t *tname) {
 }
 
 static inline isc_result_t
-dname_target(fetchctx_t *fctx, dns_rdataset_t *rdataset, dns_name_t *qname,
-	     dns_name_t *oname, dns_fixedname_t *fixeddname)
+dname_target(dns_rdataset_t *rdataset, dns_name_t *qname,
+	     unsigned int nlabels, dns_fixedname_t *fixeddname)
 {
 	isc_result_t result;
 	dns_rdata_t rdata = DNS_RDATA_INIT;
-	unsigned int nlabels;
-	int order;
-	dns_namereln_t namereln;
 	dns_rdata_dname_t dname;
 	dns_fixedname_t prefix;
 
@@ -6096,21 +6093,6 @@ dname_target(fetchctx_t *fctx, dns_rdataset_t *rdataset, dns_name_t *qname,
 	if (result != ISC_R_SUCCESS)
 		return (result);
 
-	/*
-	 * Get the prefix of qname.
-	 */
-	namereln = dns_name_fullcompare(qname, oname, &order, &nlabels);
-	if (namereln != dns_namereln_subdomain) {
-		char qbuf[DNS_NAME_FORMATSIZE];
-		char obuf[DNS_NAME_FORMATSIZE];
-
-		dns_rdata_freestruct(&dname);
-		dns_name_format(qname, qbuf, sizeof(qbuf));
-		dns_name_format(oname, obuf, sizeof(obuf));
-		log_formerr(fctx, "unrelated DNAME in answer: "
-				   "%s is not in %s", qbuf, obuf);
-		return (DNS_R_FORMERR);
-	}
 	dns_fixedname_init(&prefix);
 	dns_name_split(qname, nlabels, dns_fixedname_name(&prefix), NULL);
 	dns_fixedname_init(fixeddname);
@@ -6736,13 +6718,13 @@ static isc_result_t
 answer_response(fetchctx_t *fctx) {
 	isc_result_t result;
 	dns_message_t *message;
-	dns_name_t *name, *qname, tname, *ns_name;
+	dns_name_t *name, *dname, *qname, tname, *ns_name;
 	dns_rdataset_t *rdataset, *ns_rdataset;
 	isc_boolean_t done, external, chaining, aa, found, want_chaining;
 	isc_boolean_t have_answer, found_cname, found_type, wanted_chaining;
 	unsigned int aflag;
 	dns_rdatatype_t type;
-	dns_fixedname_t dname, fqname;
+	dns_fixedname_t fdname, fqname;
 	dns_view_t *view;
 
 	FCTXTRACE("answer_response");
@@ -6770,10 +6752,15 @@ answer_response(fetchctx_t *fctx) {
 	view = fctx->res->view;
 	result = dns_message_firstname(message, DNS_SECTION_ANSWER);
 	while (!done && result == ISC_R_SUCCESS) {
+		dns_namereln_t namereln;
+		int order;
+		unsigned int nlabels;
+
 		name = NULL;
 		dns_message_currentname(message, DNS_SECTION_ANSWER, &name);
 		external = ISC_TF(!dns_name_issubdomain(name, &fctx->domain));
-		if (dns_name_equal(name, qname)) {
+		namereln = dns_name_fullcompare(qname, name, &order, &nlabels);
+		if (namereln == dns_namereln_equal) {
 			wanted_chaining = ISC_FALSE;
 			for (rdataset = ISC_LIST_HEAD(name->list);
 			     rdataset != NULL;
@@ -6898,10 +6885,11 @@ answer_response(fetchctx_t *fctx) {
 						 */
 						INSIST(!external);
 						if (aflag ==
-						    DNS_RDATASETATTR_ANSWER)
+						    DNS_RDATASETATTR_ANSWER) {
 							have_answer = ISC_TRUE;
-						name->attributes |=
-							DNS_NAMEATTR_ANSWER;
+							name->attributes |=
+								DNS_NAMEATTR_ANSWER;
+						}
 						rdataset->attributes |= aflag;
 						if (aa)
 							rdataset->trust =
@@ -6956,6 +6944,8 @@ answer_response(fetchctx_t *fctx) {
 			if (wanted_chaining)
 				chaining = ISC_TRUE;
 		} else {
+			dns_rdataset_t *dnameset = NULL;
+
 			/*
 			 * Look for a DNAME (or its SIG).  Anything else is
 			 * ignored.
@@ -6963,10 +6953,8 @@ answer_response(fetchctx_t *fctx) {
 			wanted_chaining = ISC_FALSE;
 			for (rdataset = ISC_LIST_HEAD(name->list);
 			     rdataset != NULL;
-			     rdataset = ISC_LIST_NEXT(rdataset, link)) {
-				isc_boolean_t found_dname = ISC_FALSE;
-				dns_name_t *dname_name;
-
+			     rdataset = ISC_LIST_NEXT(rdataset, link))
+			{
 				/*
 				 * Only pass DNAME or RRSIG(DNAME).
 				 */
@@ -6980,20 +6968,41 @@ answer_response(fetchctx_t *fctx) {
 				 * its signature should not be external.
 				 */
 				if (!chaining && external) {
-					log_formerr(fctx, "external DNAME");
+					char qbuf[DNS_NAME_FORMATSIZE];
+					char obuf[DNS_NAME_FORMATSIZE];
+
+					dns_name_format(name, qbuf,
+							sizeof(qbuf));
+					dns_name_format(&fctx->domain, obuf,
+							sizeof(obuf));
+					log_formerr(fctx, "external DNAME or "
+						    "RRSIG covering DNAME "
+						    "in answer: %s is "
+						    "not in %s", qbuf, obuf);
+					return (DNS_R_FORMERR);
+				}
+
+				if (namereln != dns_namereln_subdomain) {
+					char qbuf[DNS_NAME_FORMATSIZE];
+					char obuf[DNS_NAME_FORMATSIZE];
+
+					dns_name_format(qname, qbuf,
+							sizeof(qbuf));
+					dns_name_format(name, obuf,
+							sizeof(obuf));
+					log_formerr(fctx, "unrelated DNAME "
+						    "in answer: %s is "
+						    "not in %s", qbuf, obuf);
 					return (DNS_R_FORMERR);
 				}
 
-				found = ISC_FALSE;
 				aflag = 0;
 				if (rdataset->type == dns_rdatatype_dname) {
-					found = ISC_TRUE;
 					want_chaining = ISC_TRUE;
 					POST(want_chaining);
 					aflag = DNS_RDATASETATTR_ANSWER;
-					result = dname_target(fctx, rdataset,
-							      qname, name,
-							      &dname);
+					result = dname_target(rdataset, qname,
+							      nlabels, &fdname);
 					if (result == ISC_R_NOSPACE) {
 						/*
 						 * We can't construct the
@@ -7005,14 +7014,12 @@ answer_response(fetchctx_t *fctx) {
 					} else if (result != ISC_R_SUCCESS)
 						return (result);
 					else
-						found_dname = ISC_TRUE;
+						dnameset = rdataset;
 
-					dname_name = dns_fixedname_name(&dname);
+					dname = dns_fixedname_name(&fdname);
 					if (!is_answertarget_allowed(view,
-							qname,
-							rdataset->type,
-							dname_name,
-							&fctx->domain)) {
+							qname, rdataset->type,
+							dname, &fctx->domain)) {
 						return (DNS_R_SERVFAIL);
 					}
 				} else {
@@ -7020,73 +7027,60 @@ answer_response(fetchctx_t *fctx) {
 					 * We've found a signature that
 					 * covers the DNAME.
 					 */
-					found = ISC_TRUE;
 					aflag = DNS_RDATASETATTR_ANSWERSIG;
 				}
 
-				if (found) {
+				/*
+				 * We've found an answer to our
+				 * question.
+				 */
+				name->attributes |= DNS_NAMEATTR_CACHE;
+				rdataset->attributes |= DNS_RDATASETATTR_CACHE;
+				rdataset->trust = dns_trust_answer;
+				if (!chaining) {
 					/*
-					 * We've found an answer to our
-					 * question.
+					 * This data is "the" answer to
+					 * our question only if we're
+					 * not chaining.
 					 */
-					name->attributes |=
-						DNS_NAMEATTR_CACHE;
-					rdataset->attributes |=
-						DNS_RDATASETATTR_CACHE;
-					rdataset->trust = dns_trust_answer;
-					if (!chaining) {
-						/*
-						 * This data is "the" answer
-						 * to our question only if
-						 * we're not chaining.
-						 */
-						INSIST(!external);
-						if (aflag ==
-						    DNS_RDATASETATTR_ANSWER)
-							have_answer = ISC_TRUE;
+					INSIST(!external);
+					if (aflag == DNS_RDATASETATTR_ANSWER) {
+						have_answer = ISC_TRUE;
 						name->attributes |=
 							DNS_NAMEATTR_ANSWER;
-						rdataset->attributes |= aflag;
-						if (aa)
-							rdataset->trust =
-							  dns_trust_authanswer;
-					} else if (external) {
-						rdataset->attributes |=
-						    DNS_RDATASETATTR_EXTERNAL;
-					}
-
-					/*
-					 * DNAME chaining.
-					 */
-					if (found_dname) {
-						/*
-						 * Copy the dname into the
-						 * qname fixed name.
-						 *
-						 * Although we check for
-						 * failure of the copy
-						 * operation, in practice it
-						 * should never fail since
-						 * we already know that the
-						 * result fits in a fixedname.
-						 */
-						dns_fixedname_init(&fqname);
-						result = dns_name_copy(
-						  dns_fixedname_name(&dname),
-						  dns_fixedname_name(&fqname),
-						  NULL);
-						if (result != ISC_R_SUCCESS)
-							return (result);
-						wanted_chaining = ISC_TRUE;
-						name->attributes |=
-							DNS_NAMEATTR_CHAINING;
-						rdataset->attributes |=
-						    DNS_RDATASETATTR_CHAINING;
-						qname = dns_fixedname_name(
-								   &fqname);
 					}
+					rdataset->attributes |= aflag;
+					if (aa)
+						rdataset->trust =
+						  dns_trust_authanswer;
+				} else if (external) {
+					rdataset->attributes |=
+					    DNS_RDATASETATTR_EXTERNAL;
 				}
 			}
+
+			/*
+			 * DNAME chaining.
+			 */
+			if (dnameset != NULL) {
+				/*
+				 * Copy the dname into the qname fixed name.
+				 *
+				 * Although we check for failure of the copy
+				 * operation, in practice it should never fail
+				 * since we already know that the  result fits
+				 * in a fixedname.
+				 */
+				dns_fixedname_init(&fqname);
+				qname = dns_fixedname_name(&fqname);
+				result = dns_name_copy(dname, qname, NULL);
+				if (result != ISC_R_SUCCESS)
+					return (result);
+				wanted_chaining = ISC_TRUE;
+				name->attributes |= DNS_NAMEATTR_CHAINING;
+				dnameset->attributes |=
+					    DNS_RDATASETATTR_CHAINING;
+			}
 			if (wanted_chaining)
 				chaining = ISC_TRUE;
 		}
-- 
1.9.1