Merge branch '3129-check-fetch-shutting-down-in-resume_dslookup' into 'main'

[CVE-2022-0667] Check if the fetch is shutting down in resume_dslookup()

See merge request isc-projects/bind9!5989

CHANGES

@@ -35,7 +35,9 @@
 5821.	[bug]		Fix query context management issues in the TCP part
 			of dig. [GL #3184]
 
-5820.	[placeholder]
+5820.	[security]	An assertion could occur in resume_dslookup() if the
+			fetch had been shut down earlier. (CVE-2022-0667)
+			[GL #3129]
 
 5819.	[security]	Lookups involving a DNAME could trigger an INSIST when
 			"synth-from-dnssec" was enabled. (CVE-2022-0635)

doc/notes/notes-current.rst

@@ -35,6 +35,10 @@ Security Fixes
   ISC would like to thank Vincent Levigneron from AFNIC for bringing
   this vulnerability to our attention. :gl:`#3158`
 
+- When chasing DS records, a timed-out or artificially delayed fetch
+  could cause ``named`` to crash while resuming a DS lookup.
+  (CVE-2022-0667) :gl:`#3129`
+
 Known Issues
 ~~~~~~~~~~~~
 

lib/dns/resolver.c

@@ -7242,18 +7242,21 @@ fctx__detach(fetchctx_t **fctxp, const char *file, unsigned int line,
 
 static void
 resume_dslookup(isc_task_t *task, isc_event_t *event) {
-	dns_fetchevent_t *fevent;
+	dns_fetchevent_t *fevent = NULL;
-	fetchctx_t *fctx;
+	dns_resolver_t *res = NULL;
+	fetchctx_t *fctx = NULL;
 	isc_result_t result;
 	dns_rdataset_t nameservers;
 	dns_fixedname_t fixed;
-	dns_name_t *domain;
+	dns_name_t *domain = NULL;
 	fetchctx_t *ev_fctx = NULL;
 
 	REQUIRE(event->ev_type == DNS_EVENT_FETCHDONE);
 	fevent = (dns_fetchevent_t *)event;
 	fctx = event->ev_arg;
+
 	REQUIRE(VALID_FCTX(fctx));
+	res = fctx->res;
 
 	UNUSED(task);
 	FCTXTRACE("resume_dslookup");
@@ -7282,6 +7285,15 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
 		isc_event_free(&event);
 
 		dns_resolver_destroyfetch(&fctx->nsfetch);
+
+		LOCK(&res->buckets[fctx->bucketnum].lock);
+		if (SHUTTINGDOWN(fctx)) {
+			maybe_destroy(fctx, true);
+			UNLOCK(&res->buckets[fctx->bucketnum].lock);
+			goto cleanup;
+		}
+		UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
 		fctx_done(fctx, ISC_R_CANCELED, __LINE__);
 	} else if (fevent->result == ISC_R_SUCCESS) {
 		FCTXTRACE("resuming DS lookup");
@@ -7301,6 +7313,14 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
 		fevent = NULL;
 		isc_event_free(&event);
 
+		LOCK(&res->buckets[fctx->bucketnum].lock);
+		if (SHUTTINGDOWN(fctx)) {
+			maybe_destroy(fctx, true);
+			UNLOCK(&res->buckets[fctx->bucketnum].lock);
+			goto cleanup;
+		}
+		UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
 		fcount_decr(fctx);
 		dns_name_copy(fctx->nsname, fctx->domain);
 		result = fcount_incr(fctx, true);
@@ -7329,8 +7349,17 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
 			fevent = NULL;
 			isc_event_free(&event);
 
-			fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);
 			dns_resolver_destroyfetch(&fctx->nsfetch);
+
+			LOCK(&res->buckets[fctx->bucketnum].lock);
+			if (SHUTTINGDOWN(fctx)) {
+				maybe_destroy(fctx, true);
+				UNLOCK(&res->buckets[fctx->bucketnum].lock);
+				goto cleanup;
+			}
+			UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
+			fctx_done(fctx, DNS_R_SERVFAIL, __LINE__);
 			goto cleanup;
 		}
 		if (dns_rdataset_isassociated(
@@ -7351,13 +7380,21 @@ resume_dslookup(isc_task_t *task, isc_event_t *event) {
 		fevent = NULL;
 		isc_event_free(&event);
 
+		LOCK(&res->buckets[fctx->bucketnum].lock);
+		if (SHUTTINGDOWN(fctx)) {
+			maybe_destroy(fctx, true);
+			UNLOCK(&res->buckets[fctx->bucketnum].lock);
+			goto cleanup;
+		}
+		UNLOCK(&res->buckets[fctx->bucketnum].lock);
+
 		FCTXTRACE("continuing to look for parent's NS records");
 
 		fctx_attach(fctx, &ev_fctx);
 
 		result = dns_resolver_createfetch(
-			fctx->res, fctx->nsname, dns_rdatatype_ns, domain,
+			res, fctx->nsname, dns_rdatatype_ns, domain, nsrdataset,
-			nsrdataset, NULL, NULL, 0, fctx->options, 0, NULL, task,
+			NULL, NULL, 0, fctx->options, 0, NULL, task,
 			resume_dslookup, ev_fctx, &fctx->nsrrset, NULL,
 			&fctx->nsfetch);
 		/*