From 3bcd32572504ac9b92e3c6ec1e2cee3df3b68309 Mon Sep 17 00:00:00 2001
From: Petr Mensik <pemensik@redhat.com>
Date: Tue, 20 Sep 2022 11:34:42 +0200
Subject: [PATCH 2/4] Fix CVE-2022-3080

5960.	[security]	Fix serve-stale crash that could happen when
			stale-answer-client-timeout was set to 0 and there was
			a stale CNAME in the cache for an incoming query.
			(CVE-2022-3080) [GL #3517]
---
 lib/ns/include/ns/query.h |  1 +
 lib/ns/query.c            | 42 ++++++++++++++++++++++++---------------
 2 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index 4d48cf6..34b3070 100644
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -145,6 +145,7 @@ struct query_ctx {
 	bool authoritative;		    /* authoritative query? */
 	bool want_restart;		    /* CNAME chain or other
 					     * restart needed */
+	bool		refresh_rrset;	    /* stale RRset refresh needed */
 	bool		need_wildcardproof; /* wildcard proof needed */
 	bool		nxrewrite;	    /* negative answer from RPZ */
 	bool		findcoveringnsec;   /* lookup covering NSEC */
diff --git a/lib/ns/query.c b/lib/ns/query.c
index 249321c..a450cb7 100644
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5686,7 +5686,6 @@ query_lookup(query_ctx_t *qctx) {
 	bool dbfind_stale = false;
 	bool stale_timeout = false;
 	bool stale_found = false;
-	bool refresh_rrset = false;
 	bool stale_refresh_window = false;
 
 	CCTRACE(ISC_LOG_DEBUG(3), "query_lookup");
@@ -5868,8 +5867,7 @@ query_lookup(query_ctx_t *qctx) {
 					"%s stale answer used, an attempt to "
 					"refresh the RRset will still be made",
 					namebuf);
-				refresh_rrset = STALE(qctx->rdataset);
-				qctx->client->nodetach = refresh_rrset;
+				qctx->refresh_rrset = STALE(qctx->rdataset);
 			}
 		} else {
 			/*
@@ -5907,17 +5905,6 @@ query_lookup(query_ctx_t *qctx) {
 
 	result = query_gotanswer(qctx, result);
 
-	if (refresh_rrset) {
-		/*
-		 * If we reached this point then it means that we have found a
-		 * stale RRset entry in cache and BIND is configured to allow
-		 * queries to be answered with stale data if no active RRset
-		 * is available, i.e. "stale-anwer-client-timeout 0". But, we
-		 * still need to refresh the RRset.
-		 */
-		query_refresh_rrset(qctx);
-	}
-
 cleanup:
 	return (result);
 }
@@ -7737,11 +7724,14 @@ query_addanswer(query_ctx_t *qctx) {
 
 	/*
 	 * On normal lookups, clear any rdatasets that were added on a
-	 * lookup due to stale-answer-client-timeout.
+	 * lookup due to stale-answer-client-timeout. Do not clear if we
+	 * are going to refresh the RRset, because the stale contents are
+	 * prioritized.
 	 */
 	if (QUERY_STALEOK(&qctx->client->query) &&
-	    !QUERY_STALETIMEOUT(&qctx->client->query))
+	    !QUERY_STALETIMEOUT(&qctx->client->query) && !qctx->refresh_rrset)
 	{
+		CCTRACE(ISC_LOG_DEBUG(3), "query_clear_stale");
 		query_clear_stale(qctx->client);
 		/*
 		 * We can clear the attribute to prevent redundant clearing
@@ -11457,9 +11447,29 @@ 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);
+
+	if (qctx->refresh_rrset) {
+		/*
+		 * If we reached this point then it means that we have found a
+		 * stale RRset entry in cache and BIND is configured to allow
+		 * queries to be answered with stale data if no active RRset
+		 * is available, i.e. "stale-anwer-client-timeout 0". But, we
+		 * still need to refresh the RRset. To prevent adding duplicate
+		 * RRsets, clear the RRsets from the message before doing the
+		 * refresh.
+		 */
+		message_clearrdataset(qctx->client->message, 0);
+		query_refresh_rrset(qctx);
+	}
+
 	if (!nodetach) {
 		qctx->detach_client = true;
 	}
-- 
2.37.3