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 (file)
index 0000000..dfb68b9
--- /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 dda98d921e0ee138adb8a330c926df1c1507b6ca..88dcc4d73ccedb8ba7308d6bb767bfcc99566c15 100644 (file)
--- 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;
                }