From: Artem Boldariev <artem@boldariev.com>
Date: Wed, 29 Sep 2021 15:44:20 +0000 (+0300)
Subject: Fix heap use after free when checking for "http" duplicates
X-Git-Tag: v9.17.19~20^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=ef65d3259442b77098bcb2752d01ca498edca0f5;p=thirdparty%2Fbind9.git

Fix heap use after free when checking for "http" duplicates

This commit fixes heap use after free when checking BIND's
configuration files for errors with http clauses.  The old code
was unnecessarially copying the http element name and freeing
it to early.  The name is now used directly.
---

diff --git a/bin/tests/system/checkconf/bad-doh-duplicates.conf b/bin/tests/system/checkconf/bad-doh-duplicates.conf
new file mode 100644
index 00000000000..dfb68b9b79b
--- /dev/null
+++ b/bin/tests/system/checkconf/bad-doh-duplicates.conf
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) Internet Systems Consortium, Inc. ("ISC")
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * See the COPYRIGHT file distributed with this work for additional
+ * information regarding copyright ownership.
+ */
+
+tls local-tls {
+	key-file "key.pem";
+	cert-file "cert.pem";
+};
+
+http local-http-server {
+	endpoints { "/dns-query"; };
+	listener-clients 100;
+	streams-per-connection 100;
+};
+
+# duplicated HTTP configuration
+http local-http-server {
+	endpoints { "/dns-query"; };
+	listener-clients 100;
+	streams-per-connection 100;
+};
+
+options {
+	listen-on { 10.53.0.1; };
+	http-port 80;
+	https-port 443;
+	http-listener-clients 100;
+	http-streams-per-connection 100;
+	listen-on port 443 tls local-tls http local-http-server { 10.53.0.1; };
+	listen-on port 8080 tls none http local-http-server { 10.53.0.1; };
+};
diff --git a/lib/bind9/check.c b/lib/bind9/check.c
index dda98d921e0..88dcc4d73cc 100644
--- a/lib/bind9/check.c
+++ b/lib/bind9/check.c
@@ -2031,23 +2031,22 @@ bind9_check_parentalagentlists(const cfg_obj_t *cctx, isc_log_t *logctx,
 #if HAVE_LIBNGHTTP2
 static isc_result_t
 bind9_check_httpserver(const cfg_obj_t *http, isc_log_t *logctx,
-		       isc_symtab_t *symtab, isc_mem_t *mctx) {
+		       isc_symtab_t *symtab) {
 	isc_result_t result, tresult;
 	const char *name = cfg_obj_asstring(cfg_map_getname(http));
-	char *tmp = isc_mem_strdup(mctx, name);
 	const cfg_obj_t *eps = NULL;
 	const cfg_listelt_t *elt = NULL;
 	isc_symvalue_t symvalue;
 
 	/* Check for duplicates */
 	symvalue.as_cpointer = http;
-	result = isc_symtab_define(symtab, tmp, 1, symvalue,
+	result = isc_symtab_define(symtab, name, 1, symvalue,
 				   isc_symexists_reject);
 	if (result == ISC_R_EXISTS) {
 		const char *file = NULL;
 		unsigned int line;
 
-		tresult = isc_symtab_lookup(symtab, tmp, 1, &symvalue);
+		tresult = isc_symtab_lookup(symtab, name, 1, &symvalue);
 		RUNTIME_CHECK(tresult == ISC_R_SUCCESS);
 
 		line = cfg_obj_line(symvalue.as_cpointer);
@@ -2061,7 +2060,6 @@ bind9_check_httpserver(const cfg_obj_t *http, isc_log_t *logctx,
 			    "also defined at %s:%u",
 			    name, file, line);
 	}
-	isc_mem_free(mctx, tmp);
 
 	/* Check endpoints are valid */
 	tresult = cfg_map_get(http, "endpoints", &eps);
@@ -2106,7 +2104,7 @@ bind9_check_httpservers(const cfg_obj_t *config, isc_log_t *logctx,
 
 	for (elt = cfg_list_first(obj); elt != NULL; elt = cfg_list_next(elt)) {
 		obj = cfg_listelt_value(elt);
-		tresult = bind9_check_httpserver(obj, logctx, symtab, mctx);
+		tresult = bind9_check_httpserver(obj, logctx, symtab);
 		if (result == ISC_R_SUCCESS) {
 			result = tresult;
 		}