From 9d0fa07c5e7a39db89862a4f843d2190059afb4b Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 10:58:18 +1100 Subject: [PATCH 1/3] Save the correct result value to resume with nxdomain-redirect The wrong result value was being saved for resumption with nxdomain-redirect when performing the fetch. This lead to an assert when checking that RFC 1918 reverse queries where not leaking to the global internet. --- lib/ns/query.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/lib/ns/query.c b/lib/ns/query.c index 34e47e2b1e..07537a8040 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -477,10 +477,10 @@ static void query_addnxrrsetnsec(query_ctx_t *qctx); static isc_result_t -query_nxdomain(query_ctx_t *qctx, isc_result_t res); +query_nxdomain(query_ctx_t *qctx, isc_result_t result); static isc_result_t -query_redirect(query_ctx_t *qctx); +query_redirect(query_ctx_t *qctx, isc_result_t result); static isc_result_t query_ncache(query_ctx_t *qctx, isc_result_t result); @@ -7695,8 +7695,7 @@ query_usestale(query_ctx_t *qctx, isc_result_t result) { * result from the search. */ static isc_result_t -query_gotanswer(query_ctx_t *qctx, isc_result_t res) { - isc_result_t result = res; +query_gotanswer(query_ctx_t *qctx, isc_result_t result) { char errmsg[256]; CCTRACE(ISC_LOG_DEBUG(3), "query_gotanswer"); @@ -7772,7 +7771,7 @@ root_key_sentinel: return (query_coveringnsec(qctx)); case DNS_R_NCACHENXDOMAIN: - result = query_redirect(qctx); + result = query_redirect(qctx, result); if (result != ISC_R_COMPLETE) { return (result); } @@ -9513,11 +9512,10 @@ query_addnxrrsetnsec(query_ctx_t *qctx) { * Handle NXDOMAIN and empty wildcard responses. */ static isc_result_t -query_nxdomain(query_ctx_t *qctx, isc_result_t res) { +query_nxdomain(query_ctx_t *qctx, isc_result_t result) { dns_section_t section; uint32_t ttl; - isc_result_t result = res; - bool empty_wild = (res == DNS_R_EMPTYWILD); + bool empty_wild = (result == DNS_R_EMPTYWILD); CCTRACE(ISC_LOG_DEBUG(3), "query_nxdomain"); @@ -9526,7 +9524,7 @@ query_nxdomain(query_ctx_t *qctx, isc_result_t res) { INSIST(qctx->is_zone || REDIRECT(qctx->client)); if (!empty_wild) { - result = query_redirect(qctx); + result = query_redirect(qctx, result); if (result != ISC_R_COMPLETE) { return (result); } @@ -9614,7 +9612,7 @@ cleanup: * redirecting, so query processing should continue past it. */ static isc_result_t -query_redirect(query_ctx_t *qctx) { +query_redirect(query_ctx_t *qctx, isc_result_t saved_result) { isc_result_t result; CCTRACE(ISC_LOG_DEBUG(3), "query_redirect"); @@ -9655,7 +9653,7 @@ query_redirect(query_ctx_t *qctx) { SAVE(qctx->client->query.redirect.rdataset, qctx->rdataset); SAVE(qctx->client->query.redirect.sigrdataset, qctx->sigrdataset); - qctx->client->query.redirect.result = DNS_R_NCACHENXDOMAIN; + qctx->client->query.redirect.result = saved_result; dns_name_copy(qctx->fname, qctx->client->query.redirect.fname); qctx->client->query.redirect.authoritative = qctx->authoritative; @@ -10250,7 +10248,7 @@ query_coveringnsec(query_ctx_t *qctx) { * We now have the proof that we have an NXDOMAIN. Apply * NXDOMAIN redirection if configured. */ - result = query_redirect(qctx); + result = query_redirect(qctx, DNS_R_COVERINGNSEC); if (result != ISC_R_COMPLETE) { redirected = true; goto cleanup; From 0748965b7ce7b80ff0062c7ffc661d52335c780e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 11:09:11 +1100 Subject: [PATCH 2/3] Add CHANGES note for [GL #4281] --- CHANGES | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index d3d5fa5a6e..1b27d586c0 100644 --- a/CHANGES +++ b/CHANGES @@ -4,7 +4,8 @@ 6317. [placeholder] -6316. [placeholder] +6316. [security] Specific queries could trigger an assertion check with + nxdomain-redirect enabled. (CVE-2023-5517) [GL #4281] 6315. [security] Speed up parsing of DNS messages with many different names. (CVE-2023-4408) [GL #4234] From 2fbafc2675d19136993980765c0589da599f8403 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Tue, 10 Oct 2023 11:12:24 +1100 Subject: [PATCH 3/3] Add release note for [GL #4281] --- doc/notes/notes-current.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 518a160826..f793805ae4 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -23,6 +23,10 @@ Security Fixes University, and Yuval Shavitt from Tel-Aviv University for bringing this vulnerability to our attention. :gl:`#4234` +- Specific queries could cause :iscman:`named` to crash with an + assertion failure when :any:`nxdomain-redirect` was enabled. This has + been fixed. :cve:`2023-5517` :gl:`#4281` + New Features ~~~~~~~~~~~~