Merge branch '3619-security-serve-stale-client-timeout-crash' into 'security-main'

[CVE-2022-3924] Fix the serve-stale crash when recursive clients soft quota is reached

See merge request isc-private/bind9!475

CHANGES

@@ -1,4 +1,5 @@
-6067.	[placeholder]
+6067.	[security]	Fix serve-stale crash when recursive clients soft quota
+			is reached. (CVE-2022-3924) [GL #3619]
 
 6066.	[security]	Handle RRSIG lookups when serve-stale is active.
 			(CVE-2022-3736) [GL #3622]

doc/notes/notes-current.rst

@@ -32,6 +32,15 @@ Security Fixes
   Iratxe Niño from Fundación Sarenet) for bringing this vulnerability to
   our attention. :gl:`#3622`
 
+- :iscman:`named` running as a resolver with the
+  :any:`stale-answer-client-timeout` option set to any value greater
+  than ``0`` could crash with an assertion failure, when the
+  :any:`recursive-clients` soft quota was reached. This has been fixed.
+  (CVE-2022-3924)
+
+  ISC would like to thank Maksym Odinintsev from AWS for bringing this
+  vulnerability to our attention. :gl:`#3619`
+
 New Features
 ~~~~~~~~~~~~
 

lib/dns/resolver.c

@@ -10701,6 +10701,8 @@ fail:
 void
 dns_resolver_cancelfetch(dns_fetch_t *fetch) {
 	fetchctx_t *fctx = NULL;
+	dns_fetchevent_t *event_trystale = NULL;
+	dns_fetchevent_t *event_fetchdone = NULL;
 
 	REQUIRE(DNS_FETCH_VALID(fetch));
 	fctx = fetch->private;
@@ -10711,24 +10713,69 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) {
 	LOCK(&fctx->lock);
 
 	/*
-	 * Find the completion event for this fetch (as opposed
-	 * to those for other fetches that have joined the same
-	 * fctx) and send it with result = ISC_R_CANCELED.
+	 * Find all the events associated with the provided 'fetch' (as opposed
+	 * to those for other fetches that have joined the same fctx).  The
+	 * event(s) found are only sent and removed from the fctx->events list
+	 * after this loop is finished (i.e. the fctx->events list is not
+	 * modified during iteration).
 	 */
 	for (dns_fetchevent_t *event = ISC_LIST_HEAD(fctx->events);
 	     event != NULL; event = ISC_LIST_NEXT(event, ev_link))
 	{
-		if (event->fetch == fetch) {
-			isc_task_t *task = event->ev_sender;
-			ISC_LIST_UNLINK(fctx->events, event, ev_link);
-			event->ev_sender = fctx;
-			event->result = ISC_R_CANCELED;
+		/*
+		 * Only process events associated with the provided 'fetch'.
+		 */
+		if (event->fetch != fetch) {
+			continue;
+		}
 
-			isc_task_sendanddetach(&task, ISC_EVENT_PTR(&event));
+		/*
+		 * Track various events associated with the provided 'fetch' in
+		 * separate variables as they will need to be sent in a
+		 * specific order.
+		 */
+		switch (event->ev_type) {
+		case DNS_EVENT_TRYSTALE:
+			INSIST(event_trystale == NULL);
+			event_trystale = event;
+			break;
+		case DNS_EVENT_FETCHDONE:
+			INSIST(event_fetchdone == NULL);
+			event_fetchdone = event;
+			break;
+		default:
+			UNREACHABLE();
+		}
+
+		/*
+		 * Break out of the loop once all possible types of events
+		 * associated with the provided 'fetch' are found.
+		 */
+		if (event_trystale != NULL && event_fetchdone != NULL) {
 			break;
 		}
 	}
 
+	/*
+	 * The "trystale" event must be sent before the "fetchdone" event,
+	 * because the latter clears the "recursing" query attribute, which is
+	 * required by both events (handled by the same callback function).
+	 */
+	if (event_trystale != NULL) {
+		isc_task_t *etask = event_trystale->ev_sender;
+		ISC_LIST_UNLINK(fctx->events, event_trystale, ev_link);
+		event_trystale->ev_sender = fctx;
+		event_trystale->result = ISC_R_CANCELED;
+		isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_trystale));
+	}
+	if (event_fetchdone != NULL) {
+		isc_task_t *etask = event_fetchdone->ev_sender;
+		ISC_LIST_UNLINK(fctx->events, event_fetchdone, ev_link);
+		event_fetchdone->ev_sender = fctx;
+		event_fetchdone->result = ISC_R_CANCELED;
+		isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_fetchdone));
+	}
+
 	/*
 	 * The fctx continues running even if no fetches remain;
 	 * the answer is still cached.

lib/ns/query.c

@@ -6257,7 +6257,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
 	CTRACE(ISC_LOG_DEBUG(3), "fetch_callback");
 
 	if (event->ev_type == DNS_EVENT_TRYSTALE) {
-		query_lookup_stale(client);
+		if (devent->result != ISC_R_CANCELED) {
+			query_lookup_stale(client);
+		}
 		isc_event_free(ISC_EVENT_PTR(&event));
 		return;
 	}