CVE-2019-14861: s4-rpc/dnsserver: Avoid crash in ldb_qsort() via dcesrv_DnssrvEnumRec...

dns_name_compare() had logic to put @ and the top record in the tree being
enumerated first, but if a domain had both then this would break the
older qsort() implementation in ldb_qsort() and cause a read of memory
before the base pointer.

By removing this special case (not required as the base pointer
is already seperatly located, no matter were it is in the
returned records) the crash is avoided.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138

Signed-off-by: Andrew Bartlett <abartlet@samba.org>

diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
index 353754f9261df14ac92a94a813cb8ed3fbc16feb..c0bf3425dae08fe85e6406240989891e19b7aa14 100644 (file)
--- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
+++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c
@@ -1733,6 +1733,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
        struct DNS_RPC_RECORDS_ARRAY *recs;
        char **add_names = NULL;
        char *rname;
+       const char *preference_name = NULL;
        int add_count = 0;
        int i, ret, len;
        WERROR status;
@@ -1749,6 +1750,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
                ret = ldb_search(dsstate->samdb, tmp_ctx, &res, z->zone_dn,
                                 LDB_SCOPE_ONELEVEL, attrs,
                                 "(&(objectClass=dnsNode)(!(dNSTombstoned=TRUE)))");
+               preference_name = "@";
        } else {
                char *encoded_name
                        = ldb_binary_encode_string(tmp_ctx, name);
@@ -1756,6 +1758,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
                                 LDB_SCOPE_ONELEVEL, attrs,
                                 "(&(objectClass=dnsNode)(|(name=%s)(name=*.%s))(!(dNSTombstoned=TRUE)))",
                                 encoded_name, encoded_name);
+               preference_name = name;
        }
        if (ret != LDB_SUCCESS) {
                talloc_free(tmp_ctx);
@@ -1769,16 +1772,18 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate,
        recs = talloc_zero(mem_ctx, struct DNS_RPC_RECORDS_ARRAY);
        W_ERROR_HAVE_NO_MEMORY_AND_FREE(recs, tmp_ctx);
 
-       /* Sort the names, so that the first record is the parent record */
-       ldb_qsort(res->msgs, res->count, sizeof(struct ldb_message *), name,
-                       (ldb_qsort_cmp_fn_t)dns_name_compare);
+       /*
+        * Sort the names, so that the records are in order by the child
+        * component below "name".
+        *
+        * A full tree sort is not required, so we pass in "name" so
+        * we know which level to sort, as only direct children are
+        * eventually returned
+        */
+       LDB_TYPESAFE_QSORT(res->msgs, res->count, name, dns_name_compare);
 
        /* Build a tree of name components from dns name */
-       if (strcasecmp(name, z->name) == 0) {
-               tree = dns_build_tree(tmp_ctx, "@", res);
-       } else {
-               tree = dns_build_tree(tmp_ctx, name, res);
-       }
+       tree = dns_build_tree(tmp_ctx, preference_name, res);
        W_ERROR_HAVE_NO_MEMORY_AND_FREE(tree, tmp_ctx);
 
        /* Find the parent record in the tree */

diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
index f991f4042e3a88cb16abe9231a4c8f14e269b7a7..acdb87752f8959434f0300d810a44a82ea9d4306 100644 (file)
--- a/source4/rpc_server/dnsserver/dnsdata.c
+++ b/source4/rpc_server/dnsserver/dnsdata.c
@@ -1052,8 +1052,8 @@ WERROR dns_fill_records_array(TALLOC_CTX *mem_ctx,
 }
 
 
-int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2,
-                               char *search_name)
+int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2,
+                    const char *search_name)
 {
        const char *name1, *name2;
        const char *ptr1, *ptr2;
@@ -1064,21 +1064,6 @@ int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m
                return 0;
        }
 
-       /* '@' record and the search_name record gets preference */
-       if (name1[0] == '@') {
-               return -1;
-       }
-       if (search_name && strcasecmp(name1, search_name) == 0) {
-               return -1;
-       }
-
-       if (name2[0] == '@') {
-               return 1;
-       }
-       if (search_name && strcasecmp(name2, search_name) == 0) {
-               return 1;
-       }
-
        /* Compare the last components of names.
         * If search_name is not NULL, compare the second last components of names */
        ptr1 = strrchr(name1, '.');

diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h
index 93f1d72f2ef5c4dc9751aed4984b6710b2c43198..690ab87ed109b4bb99f488a235a0e40707f863df 100644 (file)
--- a/source4/rpc_server/dnsserver/dnsserver.h
+++ b/source4/rpc_server/dnsserver/dnsserver.h
@@ -188,8 +188,8 @@ struct DNS_ADDR_ARRAY *dns_addr_array_copy(TALLOC_CTX *mem_ctx, struct DNS_ADDR_
 int dns_split_name_components(TALLOC_CTX *mem_ctx, const char *name, char ***components);
 char *dns_split_node_name(TALLOC_CTX *mem_ctx, const char *node_name, const char *zone_name);
 
-int dns_name_compare(const struct ldb_message **m1, const struct ldb_message **m2,
-                       char *search_name);
+int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const *m2,
+                    const char *search_name);
 bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRecord *rec2);
 
 void dnsp_to_dns_copy(TALLOC_CTX *mem_ctx, struct dnsp_DnssrvRpcRecord *dnsp,