Merge branch '4152-confidential-limit-isccc_cc_fromwire-recursion-depth' into 'security-main'

[CVE-2023-3341] Limit isccc_cc_fromwire recursion depth

See merge request isc-private/bind9!546

CHANGES

@@ -1,6 +1,7 @@
 6246.	[placeholder]
 
-6245.	[placeholder]
+6245.	[security]	Limit the amount of recursion that can be performed
+			by isccc_cc_fromwire. (CVE-2023-3341) [GL #4152]
 
 6244.	[bug]		Adjust log levels on malformed messages to NOTICE when
 			transferring in a zone. [GL #4290]

doc/notes/notes-current.rst

@@ -15,7 +15,13 @@ Notes for BIND 9.19.17
 Security Fixes
 ~~~~~~~~~~~~~~
 
-- None.
+- Previously, sending a specially crafted message over the control
+  channel could cause the packet-parsing code to run out of available
+  stack memory, causing :iscman:`named` to terminate unexpectedly.
+  This has been fixed. (CVE-2023-3341)
+
+  ISC would like to thank Eric Sesterhenn from X41 D-Sec GmbH for
+  bringing this vulnerability to our attention. :gl:`#4152`
 
 New Features
 ~~~~~~~~~~~~

lib/isc/include/isc/result.h

@@ -273,6 +273,7 @@ typedef enum isc_result {
 	ISCCC_R_EXPIRED,
 	ISCCC_R_CLOCKSKEW,
 	ISCCC_R_DUPLICATE,
+	ISCCC_R_MAXDEPTH,
 
 	ISC_R_NRESULTS, /*% The number of results. */
 	ISC_R_MAKE_ENUM_32BIT = INT32_MAX,

lib/isc/result.c

@@ -270,6 +270,7 @@ static const char *description[ISC_R_NRESULTS] = {
 	[ISCCC_R_EXPIRED] = "expired",
 	[ISCCC_R_CLOCKSKEW] = "clock skew",
 	[ISCCC_R_DUPLICATE] = "duplicate",
+	[ISCCC_R_MAXDEPTH] = "max depth",
 };
 
 static const char *identifier[ISC_R_NRESULTS] = {
@@ -521,6 +522,7 @@ static const char *identifier[ISC_R_NRESULTS] = {
 	[ISCCC_R_EXPIRED] = "ISCCC_R_EXPIRED",
 	[ISCCC_R_CLOCKSKEW] = "ISCCC_R_CLOCKSKEW",
 	[ISCCC_R_DUPLICATE] = "ISCCC_R_DUPLICATE",
+	[ISCCC_R_MAXDEPTH] = "ISCCC_R_MAXDEPTH",
 };
 
 STATIC_ASSERT((DNS_R_SERVFAIL - DNS_R_NOERROR == 2),

lib/isccc/cc.c

@@ -51,6 +51,10 @@
 
 #define MAX_TAGS     256
 #define DUP_LIFETIME 900
+#ifndef ISCCC_MAXDEPTH
+#define ISCCC_MAXDEPTH \
+	10 /* Big enough for rndc which just sends a string each way. */
+#endif
 
 typedef isccc_sexpr_t *sexpr_ptr;
 
@@ -483,19 +487,25 @@ verify(isccc_sexpr_t *alist, unsigned char *data, unsigned int length,
 
 static isc_result_t
 table_fromwire(isccc_region_t *source, isccc_region_t *secret,
-	       uint32_t algorithm, isccc_sexpr_t **alistp);
+	       uint32_t algorithm, unsigned int depth, isccc_sexpr_t **alistp);
 
 static isc_result_t
-list_fromwire(isccc_region_t *source, isccc_sexpr_t **listp);
+list_fromwire(isccc_region_t *source, unsigned int depth,
+	      isccc_sexpr_t **listp);
 
 static isc_result_t
-value_fromwire(isccc_region_t *source, isccc_sexpr_t **valuep) {
+value_fromwire(isccc_region_t *source, unsigned int depth,
+	       isccc_sexpr_t **valuep) {
 	unsigned int msgtype;
 	uint32_t len;
 	isccc_sexpr_t *value;
 	isccc_region_t active;
 	isc_result_t result;
 
+	if (depth > ISCCC_MAXDEPTH) {
+		return (ISCCC_R_MAXDEPTH);
+	}
+
 	if (REGION_SIZE(*source) < 1 + 4) {
 		return (ISC_R_UNEXPECTEDEND);
 	}
@@ -516,9 +526,9 @@ value_fromwire(isccc_region_t *source, isccc_sexpr_t **valuep) {
 			result = ISC_R_NOMEMORY;
 		}
 	} else if (msgtype == ISCCC_CCMSGTYPE_TABLE) {
-		result = table_fromwire(&active, NULL, 0, valuep);
+		result = table_fromwire(&active, NULL, 0, depth + 1, valuep);
 	} else if (msgtype == ISCCC_CCMSGTYPE_LIST) {
-		result = list_fromwire(&active, valuep);
+		result = list_fromwire(&active, depth + 1, valuep);
 	} else {
 		result = ISCCC_R_SYNTAX;
 	}
@@ -528,7 +538,7 @@ value_fromwire(isccc_region_t *source, isccc_sexpr_t **valuep) {
 
 static isc_result_t
 table_fromwire(isccc_region_t *source, isccc_region_t *secret,
-	       uint32_t algorithm, isccc_sexpr_t **alistp) {
+	       uint32_t algorithm, unsigned int depth, isccc_sexpr_t **alistp) {
 	char key[256];
 	uint32_t len;
 	isc_result_t result;
@@ -538,6 +548,10 @@ table_fromwire(isccc_region_t *source, isccc_region_t *secret,
 
 	REQUIRE(alistp != NULL && *alistp == NULL);
 
+	if (depth > ISCCC_MAXDEPTH) {
+		return (ISCCC_R_MAXDEPTH);
+	}
+
 	checksum_rstart = NULL;
 	first_tag = true;
 	alist = isccc_alist_create();
@@ -554,7 +568,7 @@ table_fromwire(isccc_region_t *source, isccc_region_t *secret,
 		GET_MEM(key, len, source->rstart);
 		key[len] = '\0'; /* Ensure NUL termination. */
 		value = NULL;
-		result = value_fromwire(source, &value);
+		result = value_fromwire(source, depth + 1, &value);
 		if (result != ISC_R_SUCCESS) {
 			goto bad;
 		}
@@ -592,14 +606,19 @@ bad:
 }
 
 static isc_result_t
-list_fromwire(isccc_region_t *source, isccc_sexpr_t **listp) {
+list_fromwire(isccc_region_t *source, unsigned int depth,
+	      isccc_sexpr_t **listp) {
 	isccc_sexpr_t *list, *value;
 	isc_result_t result;
 
+	if (depth > ISCCC_MAXDEPTH) {
+		return (ISCCC_R_MAXDEPTH);
+	}
+
 	list = NULL;
 	while (!REGION_EMPTY(*source)) {
 		value = NULL;
-		result = value_fromwire(source, &value);
+		result = value_fromwire(source, depth + 1, &value);
 		if (result != ISC_R_SUCCESS) {
 			isccc_sexpr_free(&list);
 			return (result);
@@ -631,7 +650,7 @@ isccc_cc_fromwire(isccc_region_t *source, isccc_sexpr_t **alistp,
 		return (ISCCC_R_UNKNOWNVERSION);
 	}
 
-	return (table_fromwire(source, secret, algorithm, alistp));
+	return (table_fromwire(source, secret, algorithm, 0, alistp));
 }
 
 static isc_result_t