diff --git a/CHANGES b/CHANGES index b34478bf5c..9bcfe6270f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +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] + 6190. [security] Improve the overmem cleaning process to prevent the cache going over the configured limit. (CVE-2023-2828) [GL #4055] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 78c3c048e2..3c706ebe98 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -24,6 +24,12 @@ Security Fixes and Yuval Shavitt from Tel-Aviv University for bringing this vulnerability to our attention. :gl:`#4055` +- 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 :iscman:`named` to enter an infinite callback + loop and crash due to stack overflow. This has been fixed. (CVE-2023-2911) + :gl:`#4089` + New Features ~~~~~~~~~~~~ diff --git a/lib/ns/query.c b/lib/ns/query.c index 1fab263a35..64e9278307 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -5827,6 +5827,7 @@ query_refresh_rrset(query_ctx_t *orig_qctx) { qctx.client->query.dboptions &= ~(DNS_DBFIND_STALETIMEOUT | DNS_DBFIND_STALEOK | DNS_DBFIND_STALEENABLED); + qctx.client->nodetach = false; /* * We'll need some resources... @@ -6100,7 +6101,14 @@ query_lookup(query_ctx_t *qctx) { "%s stale answer used, an attempt to " "refresh the RRset will still be made", namebuf); + qctx->refresh_rrset = STALE(qctx->rdataset); + /* + * If we are refreshing the RRSet, we must not + * detach from the client in query_send(). + */ + qctx->client->nodetach = qctx->refresh_rrset; + if (stale_found) { ns_client_extendederror( qctx->client, ede, @@ -6533,7 +6541,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); @@ -7650,10 +7658,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); } @@ -11950,12 +11969,7 @@ ns_query_done(query_ctx_t *qctx) { /* * Client may have been detached after query_send(), so * we test and store the flag state here, for safety. - * If we are refreshing the RRSet, we must not detach from the client - * in the query_send(), so we need to override the flag. */ - if (qctx->refresh_rrset) { - qctx->client->nodetach = true; - } nodetach = qctx->client->nodetach; query_send(qctx->client);