From 3880a89e6df11b62605014fabb4609bcea138107 Mon Sep 17 00:00:00 2001 From: Aaron Haslett Date: Tue, 23 Oct 2018 17:25:51 +1300 Subject: [PATCH 1/2] CVE-2018-14629 dns: CNAME loop prevention using counter Count number of answers generated by internal DNS query routine and stop at 20 to match Microsoft's loop prevention mechanism. BUG: https://bugzilla.samba.org/show_bug.cgi?id=13600 Signed-off-by: Aaron Haslett Reviewed-by: Andrew Bartlett Reviewed-by: Garming Sam --- python/samba/tests/dns.py | 22 ++++++++++++++++++++++ source4/dns_server/dns_query.c | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/python/samba/tests/dns.py b/python/samba/tests/dns.py index d0c2484c018..e2d9bb23ed0 100644 --- a/python/samba/tests/dns.py +++ b/python/samba/tests/dns.py @@ -846,6 +846,28 @@ class TestComplexQueries(DNSTest): self.assertEquals(response.answers[1].name, name2) self.assertEquals(response.answers[1].rdata, name0) + def test_cname_loop(self): + cname1 = "cnamelooptestrec." + self.get_dns_domain() + cname2 = "cnamelooptestrec2." + self.get_dns_domain() + cname3 = "cnamelooptestrec3." + self.get_dns_domain() + self.make_dns_update(cname1, cname2, dnsp.DNS_TYPE_CNAME) + self.make_dns_update(cname2, cname3, dnsp.DNS_TYPE_CNAME) + self.make_dns_update(cname3, cname1, dnsp.DNS_TYPE_CNAME) + + p = self.make_name_packet(dns.DNS_OPCODE_QUERY) + questions = [] + + q = self.make_name_question(cname1, + dns.DNS_QTYPE_A, + dns.DNS_QCLASS_IN) + questions.append(q) + self.finish_name_packet(p, questions) + + (response, response_packet) =\ + self.dns_transaction_udp(p, host=self.server_ip) + + max_recursion_depth = 20 + self.assertEquals(len(response.answers), max_recursion_depth) class TestInvalidQueries(DNSTest): def setUp(self): diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c index 923f7233eb9..65faeac3b6a 100644 --- a/source4/dns_server/dns_query.c +++ b/source4/dns_server/dns_query.c @@ -40,6 +40,7 @@ #undef DBGC_CLASS #define DBGC_CLASS DBGC_DNS +#define MAX_Q_RECURSION_DEPTH 20 struct forwarder_string { const char *forwarder; @@ -419,6 +420,11 @@ static struct tevent_req *handle_dnsrpcrec_send( state->answers = answers; state->nsrecs = nsrecs; + if (talloc_array_length(*answers) >= MAX_Q_RECURSION_DEPTH) { + tevent_req_done(req); + return tevent_req_post(req, ev); + } + resolve_cname = ((rec->wType == DNS_TYPE_CNAME) && ((question->question_type == DNS_QTYPE_A) || (question->question_type == DNS_QTYPE_AAAA))); -- 2.17.1 From adaff21d70f15e7120f0337c5c8c4bc0206e7482 Mon Sep 17 00:00:00 2001 From: Douglas Bagnall Date: Thu, 8 Nov 2018 11:44:31 +1300 Subject: [PATCH 2/2] fix dns: prevent self-referencing CNAME: use size_t --- source4/rpc_server/dnsserver/dcerpc_dnsserver.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c index e3ca106a075..edea2b33f2d 100644 --- a/source4/rpc_server/dnsserver/dcerpc_dnsserver.c +++ b/source4/rpc_server/dnsserver/dcerpc_dnsserver.c @@ -1876,7 +1876,7 @@ static WERROR dnsserver_enumerate_records(struct dnsserver_state *dsstate, static bool cname_self_reference(const char* node_name, const char* zone_name, struct DNS_RPC_NAME name) { - int node_len, zone_len; + size_t node_len, zone_len; if (node_name == NULL || zone_name == NULL) { return false; @@ -1885,8 +1885,8 @@ static bool cname_self_reference(const char* node_name, node_len = strlen(node_name); zone_len = strlen(zone_name); - if (node_len <= 0 || - zone_len <= 0 || + if (node_len == 0 || + zone_len == 0 || (name.len != node_len + zone_len + 1)) { return false; } @@ -1898,7 +1898,6 @@ static bool cname_self_reference(const char* node_name, } return false; - } /* dnsserver update function */ -- 2.17.1