From aa01203ff51ec49dfdfeed6ab02bbe0cb3198d70 Mon Sep 17 00:00:00 2001 From: Andrew Bartlett Date: Mon, 2 Jul 2018 16:49:37 +1200 Subject: dns_server: Be strict when constructing a LDB DN from an untrusted DNS name This changes our DNS server to be much more careful when constructing DNS names into LDB DN values. This avoids a segfault deep in the LDB code if the ldb_dn_get_casefold() fails there. A seperate patch will address that part of the issue, and a later patch will re-work this code to use single API: ldb_dn_add_child_val(). This is not squahed with this work because this patch does not rely on a new LDB release, and so may be helpful for a backport. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13466 Signed-off-by: Andrew Bartlett Reviewed-by: Douglas Bagnall --- source4/dns_server/dnsserver_common.c | 59 +++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 3 deletions(-) (limited to 'source4/dns_server') diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c index bbbfe920f4e..1204fc6bbcf 100644 --- a/source4/dns_server/dnsserver_common.c +++ b/source4/dns_server/dnsserver_common.c @@ -1056,7 +1056,11 @@ WERROR dns_common_name2dn(struct ldb_context *samdb, struct ldb_dn *dn; const struct dns_server_zone *z; size_t host_part_len = 0; + struct ldb_val host_part; WERROR werr; + bool ok; + int ret; + const char *casefold = NULL; if (name == NULL) { return DNS_ERR(FORMAT_ERROR); @@ -1065,7 +1069,13 @@ WERROR dns_common_name2dn(struct ldb_context *samdb, if (strcmp(name, "") == 0) { base = ldb_get_default_basedn(samdb); dn = ldb_dn_copy(mem_ctx, base); - ldb_dn_add_child_fmt(dn, "DC=@,DC=RootDNSServers,CN=MicrosoftDNS,CN=System"); + ok = ldb_dn_add_child_fmt(dn, + "DC=@,DC=RootDNSServers,CN=MicrosoftDNS,CN=System"); + if (ok == false) { + TALLOC_FREE(dn); + return WERR_NOT_ENOUGH_MEMORY; + } + *_dn = dn; return WERR_OK; } @@ -1091,13 +1101,56 @@ WERROR dns_common_name2dn(struct ldb_context *samdb, if (host_part_len == 0) { dn = ldb_dn_copy(mem_ctx, z->dn); - ldb_dn_add_child_fmt(dn, "DC=@"); + ok = ldb_dn_add_child_fmt(dn, "DC=@"); + if (! ok) { + TALLOC_FREE(dn); + return WERR_NOT_ENOUGH_MEMORY; + } *_dn = dn; return WERR_OK; } dn = ldb_dn_copy(mem_ctx, z->dn); - ldb_dn_add_child_fmt(dn, "DC=%*.*s", (int)host_part_len, (int)host_part_len, name); + if (dn == NULL) { + TALLOC_FREE(dn); + return WERR_NOT_ENOUGH_MEMORY; + } + + ok = ldb_dn_add_child_fmt(dn, "DC=X"); + + if (ok == false) { + TALLOC_FREE(dn); + return WERR_NOT_ENOUGH_MEMORY; + } + + host_part = data_blob_const(name, host_part_len); + + ret = ldb_dn_set_component(dn, 0, "DC", host_part); + if (ret != LDB_SUCCESS) { + TALLOC_FREE(dn); + return WERR_NOT_ENOUGH_MEMORY; + } + + /* + * Check the new DN here for validity, so as to catch errors + * early + */ + ok = ldb_dn_validate(dn); + if (ok == false) { + TALLOC_FREE(dn); + return DNS_ERR(NAME_ERROR); + } + + /* + * The value from this check is saved in the DN, and doing + * this here allows an easy return here. + */ + casefold = ldb_dn_get_casefold(dn); + if (casefold == NULL) { + TALLOC_FREE(dn); + return DNS_ERR(NAME_ERROR); + } + *_dn = dn; return WERR_OK; } -- cgit v1.2.1