From ff8a414f42225e4db7dc0393f9bccb3b2dda458b Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 21 Oct 2019 12:08:01 +1300 Subject: [PATCH 1/2] CVE-2019-14861: s4-rpc_server/dnsserver: Remove ldb_qsort() call from dnsserver_enumerate_records() The dns_name_compare() function attempts to prioritise @ and the searched domain to the top of the list but if both are found, this breaks the rules of how a comparison function should work and so breaks ldb_qsort() leading to a read of un-initialised memory. BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138 Signed-off-by: Andrew Bartlett --- source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 4 -- source4/rpc_server/dnsserver/dnsdata.c | 68 ------------------------- source4/rpc_server/dnsserver/dnsserver.h | 2 - 3 files changed, 74 deletions(-) diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c index 993e5dc4e56..0f635060f1a 100644 --- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c +++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c @@ -1799,10 +1799,6 @@ 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); - /* Build a tree of name components from dns name */ if (strcasecmp(name, z->name) == 0) { tree = dns_build_tree(tmp_ctx, "@", res); diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c index 2dc098a64a0..601f483e6b4 100644 --- a/source4/rpc_server/dnsserver/dnsdata.c +++ b/source4/rpc_server/dnsserver/dnsdata.c @@ -1065,74 +1065,6 @@ 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) -{ - const char *name1, *name2; - const char *ptr1, *ptr2; - - name1 = ldb_msg_find_attr_as_string(*m1, "name", NULL); - name2 = ldb_msg_find_attr_as_string(*m2, "name", NULL); - if (name1 == NULL || name2 == NULL) { - 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, '.'); - if (ptr1 == NULL) { - ptr1 = name1; - } else { - if (search_name && strcasecmp(ptr1+1, search_name) == 0) { - ptr1--; - while (ptr1 != name1) { - ptr1--; - if (*ptr1 == '.') { - break; - } - } - } - if (*ptr1 == '.') { - ptr1 = &ptr1[1]; - } - } - - ptr2 = strrchr(name2, '.'); - if (ptr2 == NULL) { - ptr2 = name2; - } else { - if (search_name && strcasecmp(ptr2+1, search_name) == 0) { - ptr2--; - while (ptr2 != name2) { - ptr2--; - if (*ptr2 == '.') { - break; - } - } - } - if (*ptr2 == '.') { - ptr2 = &ptr2[1]; - } - } - - return strcasecmp(ptr1, ptr2); -} - bool dns_record_match(struct dnsp_DnssrvRpcRecord *rec1, struct dnsp_DnssrvRpcRecord *rec2) { bool status; diff --git a/source4/rpc_server/dnsserver/dnsserver.h b/source4/rpc_server/dnsserver/dnsserver.h index a8307ef836a..692f9ae2aef 100644 --- a/source4/rpc_server/dnsserver/dnsserver.h +++ b/source4/rpc_server/dnsserver/dnsserver.h @@ -188,8 +188,6 @@ 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); 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, -- 2.11.0 From a7a95cfa5eea5d1a203819e381aec04cd4191533 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 21 Oct 2019 12:12:10 +1300 Subject: [PATCH 2/2] CVE-2019-14861: s4-rpc_server: Remove special case for @ in dns_build_tree() BUG: https://bugzilla.samba.org/show_bug.cgi?id=14138 Signed-off-by: Andrew Bartlett --- source4/rpc_server/dnsserver/dnsdata.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c index 601f483e6b4..35103b2d517 100644 --- a/source4/rpc_server/dnsserver/dnsdata.c +++ b/source4/rpc_server/dnsserver/dnsdata.c @@ -801,10 +801,11 @@ struct dns_tree *dns_build_tree(TALLOC_CTX *mem_ctx, const char *name, struct ld goto failed; } - if (strcmp(ptr, "@") == 0) { - base->data = res->msgs[i]; - continue; - } else if (strcasecmp(ptr, name) == 0) { + /* + * This might be the sub-domain in the zone being + * requested, or @ for the root of the zone + */ + if (strcasecmp(ptr, name) == 0) { base->data = res->msgs[i]; continue; } -- 2.11.0