From 7602be276a73a6eb5431c5acd9718e68a55e8b61 Mon Sep 17 00:00:00 2001 From: Mark Andrews 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] CVE: CVE-2016-1286 Upstream-Status: Backport (cherry picked from commit 2de89ee9de8c8da9dc153a754b02dcdbb7fe2374) Signed-off-by: Sona Sarmadi --- 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