12f1cd3444
6192. [security] A query that prioritizes stale data over lookup triggers a fetch to refresh the stale data in cache. If the fetch is aborted for exceeding the recursion quota, it was possible for 'named' to enter an infinite callback loop and crash due to stack overflow. This has been fixed. (CVE-2023-2911) [GL #4089] Resolves: CVE-2023-2911
73 lines
2.6 KiB
Diff
73 lines
2.6 KiB
Diff
From 589c06568e3036bfe713d42b53c8e88005ce17e4 Mon Sep 17 00:00:00 2001
|
|
From: Evan Hunt <each@isc.org>
|
|
Date: Thu, 25 May 2023 23:53:50 -0700
|
|
Subject: [PATCH 2/3] Stale answer lookups could loop when over recursion quota
|
|
|
|
When a query was aborted because of the recursion quota being exceeded,
|
|
but triggered a stale answer response and a stale data refresh query,
|
|
it could cause named to loop back where we are iterating and following
|
|
a delegation. Having no good answer in cache, we would fall back to
|
|
using serve-stale again, use the stale data, try to refresh the RRset,
|
|
and loop back again, without ever terminating until crashing due to
|
|
stack overflow.
|
|
|
|
This happens because in the functions 'query_notfound()' and
|
|
'query_delegation_recurse()', we check whether we can fall back to
|
|
serving stale data. We shouldn't do so if we are already refreshing
|
|
an RRset due to having prioritized stale data in cache.
|
|
|
|
In other words, we need to add an extra check to 'query_usestale()' to
|
|
disallow serving stale data if we are currently refreshing a stale
|
|
RRset.
|
|
|
|
As an additional mitigation to prevent looping, we now use the result
|
|
code ISC_R_ALREADYRUNNING rather than ISC_R_FAILURE when a recursion
|
|
loop is encountered, and we check for that condition in
|
|
'query_usestale()' as well.
|
|
|
|
(cherry picked from commit 0101e28f91fb36b6a16a0049d3b3e2b7846f23f0)
|
|
---
|
|
lib/ns/query.c | 17 ++++++++++++++---
|
|
1 file changed, 14 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/lib/ns/query.c b/lib/ns/query.c
|
|
index c169e22bf4..1eb662ea4d 100644
|
|
--- a/lib/ns/query.c
|
|
+++ b/lib/ns/query.c
|
|
@@ -6229,7 +6229,7 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
|
|
if (recparam_match(&client->query.recparam, qtype, qname, qdomain)) {
|
|
ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_QUERY,
|
|
ISC_LOG_INFO, "recursion loop detected");
|
|
- return (ISC_R_FAILURE);
|
|
+ return (ISC_R_ALREADYRUNNING);
|
|
}
|
|
|
|
recparam_update(&client->query.recparam, qtype, qname, qdomain);
|
|
@@ -7205,10 +7205,21 @@ query_usestale(query_ctx_t *qctx, isc_result_t result) {
|
|
return (false);
|
|
}
|
|
|
|
- if (result == DNS_R_DUPLICATE || result == DNS_R_DROP) {
|
|
+ if (qctx->refresh_rrset) {
|
|
+ /*
|
|
+ * This is a refreshing query, we have already prioritized
|
|
+ * stale data, so don't enable serve-stale again.
|
|
+ */
|
|
+ return (false);
|
|
+ }
|
|
+
|
|
+ if (result == DNS_R_DUPLICATE || result == DNS_R_DROP ||
|
|
+ result == ISC_R_ALREADYRUNNING)
|
|
+ {
|
|
/*
|
|
* Don't enable serve-stale if the result signals a duplicate
|
|
- * query or query that is being dropped.
|
|
+ * query or a query that is being dropped or can't proceed
|
|
+ * because of a recursion loop.
|
|
*/
|
|
return (false);
|
|
}
|
|
--
|
|
2.40.1
|
|
|