Attachment 146596 Details for Bug 224443 – Fix for BIND out-of bound read DoS sucked from upstream BIND release

[patch] Fix for BIND out-of bound read DoS sucked from upstream BIND release

bind-advisory-2007-01-25.diff (text/plain), 9.88 KB, created by Lubomir Kundrak on 2007-01-25 19:04:16 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Lubomir Kundrak

Created: 2007-01-25 19:04:16 UTC

Size: 9.88 KB

patch

obsolete

>================================================================================
>
>$ FILES="
>lib/dns/include/dns/validator.h
>lib/dns/resolver.c
>lib/dns/validator.c
>"
>$ for i in $FILES; do diff -up bind-9.4.0rc{1,2}/$i; done >bind-advisory-2007-01-25.diff
>
>================================================================================
>
>Toto su evidentne tieto dva fixy:
>
>2126.	[security]	Serialise validation of type ANY responses. [RT #16555]
>
>2124.	[security]	It was possible to dereference a freed fetch
>			context. [RT #16584]
>
>Co sa tyka dereferencie tych fetchctx, tak ked ide len o citanie, ako je uvedene
>v advisory [1], tak to nemoze okrem toho crashu narobit nejak extra velku zlobu,
>
>[1] http://marc.theaimsgroup.com/?l=bind-announce&m=116968519321296&w=2
>
>================================================================================
>
>--- bind-9.4.0rc1/lib/dns/include/dns/validator.h	2005-11-03 01:58:00.000000000 +0100
>+++ bind-9.4.0rc2/lib/dns/include/dns/validator.h	2007-01-08 03:42:00.000000000 +0100
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2004, 2005  Internet Systems Consortium, Inc. ("ISC")
>+ * Copyright (C) 2004, 2005, 2007  Internet Systems Consortium, Inc. ("ISC")
>  * Copyright (C) 2000-2003  Internet Software Consortium.
>  *
>  * Permission to use, copy, modify, and distribute this software for any
>@@ -15,7 +15,7 @@
>  * PERFORMANCE OF THIS SOFTWARE.
>  */
> 
>-/* $Id: validator.h,v 1.27.18.6 2005/11/03 00:58:00 marka Exp $ */
>+/* $Id: validator.h,v 1.27.18.8 2007/01/08 02:42:00 marka Exp $ */
> 
> #ifndef DNS_VALIDATOR_H
> #define DNS_VALIDATOR_H 1
>@@ -144,6 +144,7 @@ struct dns_validator {
>  * dns_validator_create() options.
>  */
> #define DNS_VALIDATOR_DLV 1U
>+#define DNS_VALIDATOR_DEFER 2U
> 
> ISC_LANG_BEGINDECLS
> 
>@@ -192,6 +193,15 @@ dns_validator_create(dns_view_t *view, d
>  */
> 
> void
>+dns_validator_send(dns_validator_t *validator);
>+/*%<
>+ * Send a deferred validation request
>+ *
>+ * Requires:
>+ *	'validator' to points to a valid DNSSEC validator.
>+ */
>+
>+void
> dns_validator_cancel(dns_validator_t *validator);
> /*%<
>  * Cancel a DNSSEC validation in progress.
>--- bind-9.4.0rc1/lib/dns/resolver.c	2006-12-07 07:50:34.000000000 +0100
>+++ bind-9.4.0rc2/lib/dns/resolver.c	2007-01-08 03:41:59.000000000 +0100
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2004-2006  Internet Systems Consortium, Inc. ("ISC")
>+ * Copyright (C) 2004-2007  Internet Systems Consortium, Inc. ("ISC")
>  * Copyright (C) 1999-2003  Internet Software Consortium.
>  *
>  * Permission to use, copy, modify, and distribute this software for any
>@@ -15,7 +15,7 @@
>  * PERFORMANCE OF THIS SOFTWARE.
>  */
> 
>-/* $Id: resolver.c,v 1.284.18.53 2006/12/07 06:50:34 marka Exp $ */
>+/* $Id: resolver.c,v 1.284.18.56 2007/01/08 02:41:59 marka Exp $ */
> 
> /*! \file */
> 
>@@ -223,6 +223,11 @@ struct fetchctx {
> 	dns_name_t 			nsname; 
> 	dns_fetch_t *			nsfetch;
> 	dns_rdataset_t			nsrrset;
>+
>+	/*%
>+	 * Number of queries that reference this context.
>+	 */
>+	unsigned int			nqueries;
> };
> 
> #define FCTX_MAGIC			ISC_MAGIC('F', '!', '!', '!')
>@@ -362,6 +367,7 @@ static isc_result_t ncache_adderesult(dn
> 				      dns_rdataset_t *ardataset,
> 				      isc_result_t *eresultp);
> static void validated(isc_task_t *task, isc_event_t *event); 
>+static void maybe_destroy(fetchctx_t *fctx);
> 
> static isc_result_t
> valcreate(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, dns_name_t *name,
>@@ -381,6 +387,9 @@ valcreate(fetchctx_t *fctx, dns_adbaddri
> 	valarg->fctx = fctx;
> 	valarg->addrinfo = addrinfo;
> 
>+	if (!ISC_LIST_EMPTY(fctx->validators))
>+		INSIST((valoptions & DNS_VALIDATOR_DEFER) != 0);
>+
> 	result = dns_validator_create(fctx->res->view, name, type, rdataset,
> 				      sigrdataset, fctx->rmessage,
> 				      valoptions, task, validated, valarg,
>@@ -528,6 +537,9 @@ resquery_destroy(resquery_t **queryp) {
> 
> 	INSIST(query->tcpsocket == NULL);
> 
>+	query->fctx->nqueries--;
>+	if (SHUTTINGDOWN(query->fctx))
>+		maybe_destroy(query->fctx);	/* Locks bucket. */
> 	query->magic = 0;
> 	isc_mem_put(query->mctx, query, sizeof(*query));
> 	*queryp = NULL;
>@@ -1020,6 +1032,8 @@ fctx_query(fetchctx_t *fctx, dns_adbaddr
> 	if (result != ISC_R_SUCCESS)
> 		return (result);
> 
>+	INSIST(ISC_LIST_EMPTY(fctx->validators));
>+
> 	dns_message_reset(fctx->rmessage, DNS_MESSAGE_INTENTPARSE);
> 
> 	query = isc_mem_get(res->buckets[fctx->bucketnum].mctx,
>@@ -1180,6 +1194,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddr
> 	}
> 
> 	ISC_LIST_APPEND(fctx->queries, query, link);
>+	query->fctx->nqueries++;
> 
> 	return (ISC_R_SUCCESS);
> 
>@@ -1720,7 +1735,7 @@ fctx_finddone(isc_task_t *task, isc_even
> 			want_done = ISC_TRUE;
> 		}
> 	} else if (SHUTTINGDOWN(fctx) && fctx->pending == 0 &&
>-		   ISC_LIST_EMPTY(fctx->validators)) {
>+		   fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators)) {
> 		bucketnum = fctx->bucketnum;
> 		LOCK(&res->buckets[bucketnum].lock);
> 		/*
>@@ -2550,8 +2565,8 @@ fctx_destroy(fetchctx_t *fctx) {
> 	REQUIRE(ISC_LIST_EMPTY(fctx->finds));
> 	REQUIRE(ISC_LIST_EMPTY(fctx->altfinds));
> 	REQUIRE(fctx->pending == 0);
>-	REQUIRE(ISC_LIST_EMPTY(fctx->validators));
> 	REQUIRE(fctx->references == 0);
>+	REQUIRE(ISC_LIST_EMPTY(fctx->validators));
> 
> 	FCTXTRACE("destroy");
> 
>@@ -2741,7 +2756,7 @@ fctx_doshutdown(isc_task_t *task, isc_ev
> 	}
> 
> 	if (fctx->references == 0 && fctx->pending == 0 &&
>-	    ISC_LIST_EMPTY(fctx->validators))
>+	    fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators))
> 		bucket_empty = fctx_destroy(fctx);
> 
> 	UNLOCK(&res->buckets[bucketnum].lock);
>@@ -2782,6 +2797,7 @@ fctx_start(isc_task_t *task, isc_event_t
> 		 * pending ADB finds and no pending validations.
> 		 */
> 		INSIST(fctx->pending == 0);
>+		INSIST(fctx->nqueries == 0);
> 		INSIST(ISC_LIST_EMPTY(fctx->validators));
> 		if (fctx->references == 0) {
> 			/*
>@@ -2948,6 +2964,7 @@ fctx_create(dns_resolver_t *res, dns_nam
> 	fctx->timeouts = 0;
> 	fctx->attributes = 0;
> 	fctx->spilled = ISC_FALSE;
>+	fctx->nqueries = 0;
> 
> 	dns_name_init(&fctx->nsname, NULL);
> 	fctx->nsfetch = NULL;
>@@ -3278,12 +3295,21 @@ maybe_destroy(fetchctx_t *fctx) {
> 	unsigned int bucketnum;
> 	isc_boolean_t bucket_empty = ISC_FALSE;
> 	dns_resolver_t *res = fctx->res;
>+	dns_validator_t *validator;
> 
> 	REQUIRE(SHUTTINGDOWN(fctx));
> 
>-	if (fctx->pending != 0 || !ISC_LIST_EMPTY(fctx->validators))
>+	if (fctx->pending != 0 || fctx->nqueries != 0)
> 		return;
> 
>+	for (validator = ISC_LIST_HEAD(fctx->validators);
>+	     validator != NULL;
>+	     validator = ISC_LIST_HEAD(fctx->validators)) {
>+		ISC_LIST_UNLINK(fctx->validators, validator, link);
>+		dns_validator_cancel(validator);
>+		dns_validator_destroy(&validator);
>+	}
>+
> 	bucketnum = fctx->bucketnum;
> 	LOCK(&res->buckets[bucketnum].lock);
> 	if (fctx->references == 0)
>@@ -3418,7 +3444,9 @@ validated(isc_task_t *task, isc_event_t 
> 		add_bad(fctx, &addrinfo->sockaddr, result);
> 		isc_event_free(&event);
> 		UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock);
>-		if (sentresponse)
>+		if (!ISC_LIST_EMPTY(fctx->validators))
>+			dns_validator_send(ISC_LIST_HEAD(fctx->validators));
>+		else if (sentresponse)
> 			fctx_done(fctx, result);	/* Locks bucket. */
> 		else
> 			fctx_try(fctx);			/* Locks bucket. */
>@@ -3517,6 +3545,7 @@ validated(isc_task_t *task, isc_event_t 
> 		 * be validated.
> 		 */
> 		UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock);
>+		dns_validator_send(ISC_LIST_HEAD(fctx->validators));
> 		goto cleanup_event;
> 	}
> 
>@@ -3829,6 +3858,13 @@ cache_name(fetchctx_t *fctx, dns_name_t 
> 							   rdataset,
> 							   sigrdataset,
> 							   valoptions, task);
>+					/*
>+					 * Defer any further validations.
>+					 * This prevents multiple validators
>+					 * from manipulating fctx->rmessage
>+					 * simultaniously.
>+					 */
>+					valoptions |= DNS_VALIDATOR_DEFER;
> 				}
> 			} else if (CHAINING(rdataset)) {
> 				if (rdataset->type == dns_rdatatype_cname)
>@@ -6728,7 +6764,8 @@ dns_resolver_destroyfetch(dns_fetch_t **
> 		/*
> 		 * No one cares about the result of this fetch anymore.
> 		 */
>-		if (fctx->pending == 0 && ISC_LIST_EMPTY(fctx->validators) &&
>+		if (fctx->pending == 0 && fctx->nqueries == 0 &&
>+		    ISC_LIST_EMPTY(fctx->validators) &&
> 		    SHUTTINGDOWN(fctx)) {
> 			/*
> 			 * This fctx is already shutdown; we were just
>--- bind-9.4.0rc1/lib/dns/validator.c	2006-12-07 07:50:34.000000000 +0100
>+++ bind-9.4.0rc2/lib/dns/validator.c	2007-01-08 03:41:59.000000000 +0100
>@@ -1,5 +1,5 @@
> /*
>- * Copyright (C) 2004-2006  Internet Systems Consortium, Inc. ("ISC")
>+ * Copyright (C) 2004-2007  Internet Systems Consortium, Inc. ("ISC")
>  * Copyright (C) 2000-2003  Internet Software Consortium.
>  *
>  * Permission to use, copy, modify, and distribute this software for any
>@@ -15,7 +15,7 @@
>  * PERFORMANCE OF THIS SOFTWARE.
>  */
> 
>-/* $Id: validator.c,v 1.119.18.27 2006/12/07 06:50:34 marka Exp $ */
>+/* $Id: validator.c,v 1.119.18.29 2007/01/08 02:41:59 marka Exp $ */
> 
> /*! \file */
> 
>@@ -2877,7 +2877,8 @@ dns_validator_create(dns_view_t *view, d
> 	ISC_LINK_INIT(val, link);
> 	val->magic = VALIDATOR_MAGIC;
> 
>-	isc_task_send(task, ISC_EVENT_PTR(&event));
>+	if ((options & DNS_VALIDATOR_DEFER) == 0)
>+		isc_task_send(task, ISC_EVENT_PTR(&event));
> 
> 	*validatorp = val;
> 
>@@ -2895,6 +2896,21 @@ dns_validator_create(dns_view_t *view, d
> }
> 
> void
>+dns_validator_send(dns_validator_t *validator) {
>+	isc_event_t *event;
>+	REQUIRE(VALID_VALIDATOR(validator));
>+
>+	LOCK(&validator->lock);
>+
>+	INSIST((validator->options & DNS_VALIDATOR_DEFER) != 0);
>+	event = (isc_event_t *)validator->event;
>+	validator->options &= ~DNS_VALIDATOR_DEFER;
>+	UNLOCK(&validator->lock);
>+
>+	isc_task_send(validator->task, ISC_EVENT_PTR(&event));
>+}
>+
>+void
> dns_validator_cancel(dns_validator_t *validator) {
> 	REQUIRE(VALID_VALIDATOR(validator));
> 
>@@ -2908,6 +2924,12 @@ dns_validator_cancel(dns_validator_t *va
> 
> 		if (validator->subvalidator != NULL)
> 			dns_validator_cancel(validator->subvalidator);
>+		if ((validator->options & DNS_VALIDATOR_DEFER) != 0) {
>+			isc_task_t *task = validator->event->ev_sender;
>+			validator->options &= ~DNS_VALIDATOR_DEFER;
>+			isc_event_free((isc_event_t **)&validator->event);
>+			isc_task_detach(&task);
>+		}
> 	}
> 	UNLOCK(&validator->lock);
> }

Actions: View | Diff

Attachments on bug 224443: 146596