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