From ef65d3259442b77098bcb2752d01ca498edca0f5 Mon Sep 17 00:00:00 2001 From: Artem Boldariev Date: Wed, 29 Sep 2021 18:44:20 +0300 Subject: [PATCH] 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. --- .../system/checkconf/bad-doh-duplicates.conf | 38 +++++++++++++++++++ lib/bind9/check.c | 10 ++--- 2 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 bin/tests/system/checkconf/bad-doh-duplicates.conf 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 0000000000..dfb68b9b79 --- /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 dda98d921e..88dcc4d73c 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; }