The Samba-Bugzilla – Attachment 16015 Details for
Bug 14378
CVE-2020-10745 [SECURITY] invalid DNS or NBT queries containing dots use several seconds of CPU each
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch for master v3 -- with tests
cve-2020-10745-v3.patch (text/plain), 26.84 KB, created by
Douglas Bagnall
on 2020-06-03 00:58:22 UTC
(
hide
)
Description:
patch for master v3 -- with tests
Filename:
MIME Type:
Creator:
Douglas Bagnall
Created:
2020-06-03 00:58:22 UTC
Size:
26.84 KB
patch
obsolete
>From 8d94dee73b35110ec75ec7f7555654166a1cbfc8 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 1 May 2020 23:11:47 +1200 >Subject: [PATCH 1/6] CVE-2020-10745: pytests: hand-rolled invalid dns/nbt > packet tests > >The client libraries don't allow us to make packets that are broken in >certain ways, so we need to construct them as byte strings. > >These tests all fail at present, proving the server is rendered >unresponsive, which is the crux of CVE-2020-10745. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > python/samba/tests/dns_packet.py | 193 +++++++++++++++++++++++++++++++ > selftest/knownfail.d/dns_packet | 4 + > source4/selftest/tests.py | 10 ++ > 3 files changed, 207 insertions(+) > create mode 100644 python/samba/tests/dns_packet.py > create mode 100644 selftest/knownfail.d/dns_packet > >diff --git a/python/samba/tests/dns_packet.py b/python/samba/tests/dns_packet.py >new file mode 100644 >index 00000000000..2b3c37a8ab6 >--- /dev/null >+++ b/python/samba/tests/dns_packet.py >@@ -0,0 +1,193 @@ >+"""Sanity tests for DNS and NBT server parsing. >+ >+We don't use a proper client library so we can make improper packets. >+""" >+ >+import os >+import struct >+import socket >+import select >+from samba.dcerpc import dns, nbt >+ >+from samba.tests import TestCase >+ >+ >+def _msg_id(): >+ while True: >+ for i in range(1, 0xffff): >+ yield i >+ >+ >+SERVER = os.environ['SERVER_IP'] >+SERVER_NAME = f"{os.environ['SERVER']}.{os.environ['REALM']}" >+TIMEOUT = 0.5 >+ >+ >+def encode_netbios_bytes(chars): >+ """Even RFC 1002 uses distancing quotes when calling this "compression".""" >+ out = [] >+ chars = (chars + b' ')[:16] >+ for c in chars: >+ out.append((c >> 4) + 65) >+ out.append((c & 15) + 65) >+ return bytes(out) >+ >+ >+class TestDnsPacketBase(TestCase): >+ msg_id = _msg_id() >+ >+ def tearDown(self): >+ # we need to ensure the DNS server is responsive before >+ # continuing. >+ for i in range(40): >+ ok = self._known_good_query() >+ if ok: >+ return >+ print(f"the server is STILL unresponsive after {40 * TIMEOUT} seconds") >+ >+ def decode_reply(self, data): >+ header = data[:12] >+ id, flags, n_q, n_a, n_rec, n_exta = struct.unpack('!6H', >+ header) >+ return { >+ 'rcode': flags & 0xf >+ } >+ >+ def construct_query(self, names): >+ """Create a query packet containing one query record. >+ >+ *names* is either a single string name in the usual dotted >+ form, or a list of names. In the latter case, each name can >+ be a dotted string or a list of byte components, which allows >+ dots in components. Where I say list, I mean non-string >+ iterable. >+ >+ Examples: >+ >+ # these 3 are all the same >+ "example.com" >+ ["example.com"] >+ [[b"example", b"com"]] >+ >+ # this is three names in the same request >+ ["example.com", >+ [b"example", b"com", b"..!"], >+ (b"first component", b" 2nd component")] >+ """ >+ header = struct.pack('!6H', >+ next(self.msg_id), >+ 0x0100, # query, with recursion >+ len(names), # number of queries >+ 0x0000, # no answers >+ 0x0000, # no records >+ 0x0000, # no extra records >+ ) >+ tail = struct.pack('!BHH', >+ 0x00, # root node >+ self.qtype, >+ 0x0001, # class IN-ternet >+ ) >+ encoded_bits = [] >+ for name in names: >+ if isinstance(name, str): >+ bits = name.encode('utf8').split(b'.') >+ else: >+ bits = name >+ >+ for b in bits: >+ encoded_bits.append(b'%c%s' % (len(b), b)) >+ encoded_bits.append(tail) >+ >+ return header + b''.join(encoded_bits) >+ >+ def _test_query(self, names=(), expected_rcode=None): >+ >+ if isinstance(names, str): >+ names = [names] >+ >+ packet = self.construct_query(names) >+ s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) >+ s.sendto(packet, self.server) >+ r, _, _ = select.select([s], [], [], TIMEOUT) >+ s.close() >+ # It is reasonable to not reply to these packets (Windows >+ # doesn't), but it is not reasonable to render the server >+ # unresponsive. >+ if r != [s]: >+ ok = self._known_good_query() >+ self.assertTrue(ok, f"the server is unresponsive") >+ >+ def _known_good_query(self): >+ if self.server[1] == 53: >+ name = SERVER_NAME >+ expected_rcode = dns.DNS_RCODE_OK >+ else: >+ name = [encode_netbios_bytes(b'nxdomain'), b'nxdomain'] >+ expected_rcode = nbt.NBT_RCODE_NAM >+ >+ packet = self.construct_query([name]) >+ s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) >+ s.sendto(packet, self.server) >+ r, _, _ = select.select([s], [], [], TIMEOUT) >+ if r != [s]: >+ s.close() >+ return False >+ >+ data, addr = s.recvfrom(4096) >+ s.close() >+ rcode = self.decode_reply(data)['rcode'] >+ return expected_rcode == rcode >+ >+ >+class TestDnsPackets(TestDnsPacketBase): >+ server = (SERVER, 53) >+ qtype = 1 # dns type A >+ >+ def _test_many_repeated_components(self, label, n, expected_rcode=None): >+ name = [label] * n >+ self._test_query([name], >+ expected_rcode=expected_rcode) >+ >+ def test_127_very_dotty_components(self): >+ label = b'.' * 63 >+ self._test_many_repeated_components(label, 127) >+ >+ def test_127_half_dotty_components(self): >+ label = b'x.' * 31 + b'x' >+ self._test_many_repeated_components(label, 127) >+ >+ >+class TestNbtPackets(TestDnsPacketBase): >+ server = (SERVER, 137) >+ qtype = 0x20 # NBT_QTYPE_NETBIOS >+ >+ def _test_nbt_encode_query(self, names, *args, **kwargs): >+ if isinstance(names, str): >+ names = [names] >+ >+ nbt_names = [] >+ for name in names: >+ if isinstance(name, str): >+ bits = name.encode('utf8').split(b'.') >+ else: >+ bits = name >+ >+ encoded = [encode_netbios_bytes(bits[0])] >+ encoded.extend(bits[1:]) >+ nbt_names.append(encoded) >+ >+ self._test_query(nbt_names, *args, **kwargs) >+ >+ def _test_many_repeated_components(self, label, n, expected_rcode=None): >+ name = [label] * n >+ name[0] = encode_netbios_bytes(label) >+ self._test_query([name], >+ expected_rcode=expected_rcode) >+ >+ def test_127_very_dotty_components(self): >+ label = b'.' * 63 >+ self._test_many_repeated_components(label, 127) >+ >+ def test_127_half_dotty_components(self): >+ label = b'x.' * 31 + b'x' >+ self._test_many_repeated_components(label, 127) >diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet >new file mode 100644 >index 00000000000..fedca760912 >--- /dev/null >+++ b/selftest/knownfail.d/dns_packet >@@ -0,0 +1,4 @@ >+samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components >+samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components >+samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components >+samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components >diff --git a/source4/selftest/tests.py b/source4/selftest/tests.py >index 211a56e689a..ddb8a12b8d6 100755 >--- a/source4/selftest/tests.py >+++ b/source4/selftest/tests.py >@@ -454,6 +454,16 @@ plantestsuite_loadlist("samba.tests.dns_wildcard", "ad_dc", [python, os.path.joi > > plantestsuite_loadlist("samba.tests.dns_invalid", "ad_dc", [python, os.path.join(srcdir(), "python/samba/tests/dns_invalid.py"), '$SERVER_IP', '--machine-pass', '-U"$USERNAME%$PASSWORD"', '--workgroup=$DOMAIN', '$LOADLIST', '$LISTOPT']) > >+plantestsuite_loadlist("samba.tests.dns_packet", >+ "ad_dc", >+ [python, >+ '-msamba.subunit.run', >+ '$LOADLIST', >+ "$LISTOPT" >+ "samba.tests.dns_packet" >+ ]) >+ >+ > for t in smbtorture4_testsuites("dns_internal."): > plansmbtorture4testsuite(t, "ad_dc_default:local", '//$SERVER/whavever') > >-- >2.17.1 > > >From b66454c9ec117463b54ad8da5e88598f48b51a1d Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Sat, 25 Apr 2020 11:02:08 +1200 >Subject: [PATCH 2/6] CVE-2020-10745: ndr_dns: move ndr_push_dns_string core > into sharable function > >This is because ndr_nbt.c does almost exactly the same thing with >almost exactly the same code, and they both do it wrong. Soon they >will both be using the better version that this will become. Though in >this patch we just move the code, not fix it. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns.c | 79 +++------------------------------- > librpc/ndr/ndr_dns_utils.c | 88 ++++++++++++++++++++++++++++++++++++++ > librpc/ndr/ndr_dns_utils.h | 5 +++ > librpc/wscript_build | 2 +- > 4 files changed, 99 insertions(+), 75 deletions(-) > create mode 100644 librpc/ndr/ndr_dns_utils.c > create mode 100644 librpc/ndr/ndr_dns_utils.h > >diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c >index d37c8cc2ece..68a3c9de782 100644 >--- a/librpc/ndr/ndr_dns.c >+++ b/librpc/ndr/ndr_dns.c >@@ -33,6 +33,7 @@ > #include "librpc/gen_ndr/ndr_dnsp.h" > #include "system/locale.h" > #include "lib/util/util_net.h" >+#include "ndr_dns_utils.h" > > /* don't allow an unlimited number of name components */ > #define MAX_COMPONENTS 128 >@@ -159,80 +160,10 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, > int ndr_flags, > const char *s) > { >- if (!(ndr_flags & NDR_SCALARS)) { >- return NDR_ERR_SUCCESS; >- } >- >- while (s && *s) { >- enum ndr_err_code ndr_err; >- char *compname; >- size_t complen; >- uint32_t offset; >- >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >- /* see if we have pushed the remaining string already, >- * if so we use a label pointer to this string >- */ >- ndr_err = ndr_token_retrieve_cmp_fn(&ndr->dns_string_list, s, >- &offset, >- (comparison_fn_t)strcmp, >- false); >- if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- uint8_t b[2]; >- >- if (offset > 0x3FFF) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "offset for dns string " \ >- "label pointer " \ >- "%u[%08X] > 0x00003FFF", >- offset, offset); >- } >- >- b[0] = 0xC0 | (offset>>8); >- b[1] = (offset & 0xFF); >- >- return ndr_push_bytes(ndr, b, 2); >- } >- } >- >- complen = strcspn(s, "."); >- >- /* we need to make sure the length fits into 6 bytes */ >- if (complen > 0x3F) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "component length %u[%08X] > " \ >- "0x0000003F", >- (unsigned)complen, >- (unsigned)complen); >- } >- >- compname = talloc_asprintf(ndr, "%c%*.*s", >- (unsigned char)complen, >- (unsigned char)complen, >- (unsigned char)complen, s); >- NDR_ERR_HAVE_NO_MEMORY(compname); >- >- /* remember the current component + the rest of the string >- * so it can be reused later >- */ >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >- NDR_CHECK(ndr_token_store(ndr, &ndr->dns_string_list, s, >- ndr->offset)); >- } >- >- /* push just this component into the blob */ >- NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, >- complen+1)); >- talloc_free(compname); >- >- s += complen; >- if (*s == '.') s++; >- } >- >- /* if we reach the end of the string and have pushed the last component >- * without using a label pointer, we need to terminate the string >- */ >- return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+ return ndr_push_dns_string_list(ndr, >+ &ndr->dns_string_list, >+ ndr_flags, >+ s); > } > > _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >new file mode 100644 >index 00000000000..2d9b5f1bc1e >--- /dev/null >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -0,0 +1,88 @@ >+#include "includes.h" >+#include "../librpc/ndr/libndr.h" >+#include "ndr_dns_utils.h" >+ >+ >+/** >+ push a dns/nbt string list to the wire >+*/ >+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, >+ struct ndr_token_list *string_list, >+ int ndr_flags, >+ const char *s) >+{ >+ if (!(ndr_flags & NDR_SCALARS)) { >+ return NDR_ERR_SUCCESS; >+ } >+ >+ while (s && *s) { >+ enum ndr_err_code ndr_err; >+ char *compname; >+ size_t complen; >+ uint32_t offset; >+ >+ if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ /* see if we have pushed the remaining string already, >+ * if so we use a label pointer to this string >+ */ >+ ndr_err = ndr_token_retrieve_cmp_fn(string_list, s, >+ &offset, >+ (comparison_fn_t)strcmp, >+ false); >+ if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >+ uint8_t b[2]; >+ >+ if (offset > 0x3FFF) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "offset for dns string " \ >+ "label pointer " \ >+ "%u[%08X] > 0x00003FFF", >+ offset, offset); >+ } >+ >+ b[0] = 0xC0 | (offset>>8); >+ b[1] = (offset & 0xFF); >+ >+ return ndr_push_bytes(ndr, b, 2); >+ } >+ } >+ >+ complen = strcspn(s, "."); >+ >+ /* we need to make sure the length fits into 6 bytes */ >+ if (complen > 0x3F) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "component length %u[%08X] > " \ >+ "0x0000003F", >+ (unsigned)complen, >+ (unsigned)complen); >+ } >+ >+ compname = talloc_asprintf(ndr, "%c%*.*s", >+ (unsigned char)complen, >+ (unsigned char)complen, >+ (unsigned char)complen, s); >+ NDR_ERR_HAVE_NO_MEMORY(compname); >+ >+ /* remember the current component + the rest of the string >+ * so it can be reused later >+ */ >+ if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ NDR_CHECK(ndr_token_store(ndr, string_list, s, >+ ndr->offset)); >+ } >+ >+ /* push just this component into the blob */ >+ NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, >+ complen+1)); >+ talloc_free(compname); >+ >+ s += complen; >+ if (*s == '.') s++; >+ } >+ >+ /* if we reach the end of the string and have pushed the last component >+ * without using a label pointer, we need to terminate the string >+ */ >+ return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+} >diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h >new file mode 100644 >index 00000000000..823e3201112 >--- /dev/null >+++ b/librpc/ndr/ndr_dns_utils.h >@@ -0,0 +1,5 @@ >+ >+enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, >+ struct ndr_token_list *string_list, >+ int ndr_flags, >+ const char *s); >diff --git a/librpc/wscript_build b/librpc/wscript_build >index aa112890a7a..c5f676cd3b6 100644 >--- a/librpc/wscript_build >+++ b/librpc/wscript_build >@@ -31,7 +31,7 @@ bld.SAMBA_SUBSYSTEM('NDR_DNSSERVER', > ) > > bld.SAMBA_SUBSYSTEM('NDR_DNS', >- source='gen_ndr/ndr_dns.c ndr/ndr_dns.c', >+ source='gen_ndr/ndr_dns.c ndr/ndr_dns.c ndr/ndr_dns_utils.c', > public_deps='ndr NDR_DNSP' > ) > >-- >2.17.1 > > >From ec8b7529e4bddf22012b38e91c4492b8c8bb2876 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Sat, 25 Apr 2020 11:10:18 +1200 >Subject: [PATCH 3/6] CVE-2020-10745: ndr_dns: do not allow consecutive dots > >The empty subdomain component is reserved for the root domain, which we >should only (and always) see at the end of the list. That is, we expect >"example.com.", but never "example..com". > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns_utils.c | 6 ++++++ > selftest/knownfail.d/dns_packet | 1 - > 2 files changed, 6 insertions(+), 1 deletion(-) > >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index 2d9b5f1bc1e..67cada217a7 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -58,6 +58,12 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > (unsigned)complen); > } > >+ if (complen == 0 && s[complen] == '.') { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "component length is 0 " >+ "(consecutive dots)"); >+ } >+ > compname = talloc_asprintf(ndr, "%c%*.*s", > (unsigned char)complen, > (unsigned char)complen, >diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet >index fedca760912..bb176f72d5d 100644 >--- a/selftest/knownfail.d/dns_packet >+++ b/selftest/knownfail.d/dns_packet >@@ -1,4 +1,3 @@ > samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components >-samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_very_dotty_components > samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components > samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components >-- >2.17.1 > > >From e8baf247325789fc832520854437fc28cda34ca4 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 00:06:08 +1200 >Subject: [PATCH 4/6] CVE-2020-10745: dns_util/push: forbid names longer than > 255 bytes > >As per RFC 1035. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns_utils.c | 10 +++++++++- > selftest/knownfail.d/dns_packet | 1 - > 2 files changed, 9 insertions(+), 2 deletions(-) > >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index 67cada217a7..ab59c9853f5 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -11,6 +11,8 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > int ndr_flags, > const char *s) > { >+ const char *start = s; >+ > if (!(ndr_flags & NDR_SCALARS)) { > return NDR_ERR_SUCCESS; > } >@@ -84,7 +86,13 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > talloc_free(compname); > > s += complen; >- if (*s == '.') s++; >+ if (*s == '.') { >+ s++; >+ } >+ if (s - start > 255) { >+ return ndr_push_error(ndr, NDR_ERR_STRING, >+ "name > 255 character long"); >+ } > } > > /* if we reach the end of the string and have pushed the last component >diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet >index bb176f72d5d..da7347d5af3 100644 >--- a/selftest/knownfail.d/dns_packet >+++ b/selftest/knownfail.d/dns_packet >@@ -1,3 +1,2 @@ >-samba.tests.dns_packet.samba.tests.dns_packet.TestDnsPackets.test_127_half_dotty_components > samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components > samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components >-- >2.17.1 > > >From 32f73624f7de08517b0bad79147eba5f24ba0ae9 Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 10:52:45 +1200 >Subject: [PATCH 5/6] CVE-2020-10745: ndr/dns-utils: prepare for NBT > compatibility > >NBT has a funny thing where it sometimes needs to send a trailing dot as >part of the last component, because the string representation is a user >name. In DNS, "example.com", and "example.com." are the same, both >having three components ("example", "com", ""); in NBT, we want to treat >them differently, with the second form having the three components >("example", "com.", ""). > >This retains the logic of e6e2ec0001fe3c010445e26cc0efddbc1f73416b. > >Also DNS compression cannot be turned off for NBT. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_dns.c | 3 ++- > librpc/ndr/ndr_dns_utils.c | 21 ++++++++++++++++++--- > librpc/ndr/ndr_dns_utils.h | 3 ++- > 3 files changed, 22 insertions(+), 5 deletions(-) > >diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c >index 68a3c9de782..966e0b59786 100644 >--- a/librpc/ndr/ndr_dns.c >+++ b/librpc/ndr/ndr_dns.c >@@ -163,7 +163,8 @@ _PUBLIC_ enum ndr_err_code ndr_push_dns_string(struct ndr_push *ndr, > return ndr_push_dns_string_list(ndr, > &ndr->dns_string_list, > ndr_flags, >- s); >+ s, >+ false); > } > > _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r) >diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c >index ab59c9853f5..eb0c9ab9026 100644 >--- a/librpc/ndr/ndr_dns_utils.c >+++ b/librpc/ndr/ndr_dns_utils.c >@@ -9,9 +9,16 @@ > enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > struct ndr_token_list *string_list, > int ndr_flags, >- const char *s) >+ const char *s, >+ bool is_nbt) > { > const char *start = s; >+ bool use_compression; >+ if (is_nbt) { >+ use_compression = true; >+ } else { >+ use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION); >+ } > > if (!(ndr_flags & NDR_SCALARS)) { > return NDR_ERR_SUCCESS; >@@ -23,7 +30,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > size_t complen; > uint32_t offset; > >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ if (use_compression) { > /* see if we have pushed the remaining string already, > * if so we use a label pointer to this string > */ >@@ -66,6 +73,14 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > "(consecutive dots)"); > } > >+ if (is_nbt && s[complen] == '.' && s[complen + 1] == '\0') { >+ /* nbt names are sometimes usernames, and we need to >+ * keep a trailing dot to ensure it is byte-identical, >+ * (not just semantically identical given DNS >+ * semantics). */ >+ complen++; >+ } >+ > compname = talloc_asprintf(ndr, "%c%*.*s", > (unsigned char)complen, > (unsigned char)complen, >@@ -75,7 +90,7 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > /* remember the current component + the rest of the string > * so it can be reused later > */ >- if (!(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION)) { >+ if (use_compression) { > NDR_CHECK(ndr_token_store(ndr, string_list, s, > ndr->offset)); > } >diff --git a/librpc/ndr/ndr_dns_utils.h b/librpc/ndr/ndr_dns_utils.h >index 823e3201112..71a65433bbb 100644 >--- a/librpc/ndr/ndr_dns_utils.h >+++ b/librpc/ndr/ndr_dns_utils.h >@@ -2,4 +2,5 @@ > enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr, > struct ndr_token_list *string_list, > int ndr_flags, >- const char *s); >+ const char *s, >+ bool is_nbt); >-- >2.17.1 > > >From bb15db6abf811e71ee319359893f6ce0f99c0a6a Mon Sep 17 00:00:00 2001 >From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >Date: Fri, 15 May 2020 11:48:27 +1200 >Subject: [PATCH 6/6] CVE-2020-10745: ndr/nbt_push_string: use dns_utils common > function > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378 > >Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz> >--- > librpc/ndr/ndr_nbt.c | 72 ++++----------------------------- > librpc/wscript_build | 2 +- > selftest/knownfail.d/dns_packet | 2 - > 3 files changed, 8 insertions(+), 68 deletions(-) > delete mode 100644 selftest/knownfail.d/dns_packet > >diff --git a/librpc/ndr/ndr_nbt.c b/librpc/ndr/ndr_nbt.c >index b15dc5c06e5..8ed9f0a5f05 100644 >--- a/librpc/ndr/ndr_nbt.c >+++ b/librpc/ndr/ndr_nbt.c >@@ -25,6 +25,8 @@ > #include "includes.h" > #include "../libcli/nbt/libnbt.h" > #include "../libcli/netlogon/netlogon.h" >+#include "ndr_dns_utils.h" >+ > > /* don't allow an unlimited number of name components */ > #define MAX_COMPONENTS 128 >@@ -141,71 +143,11 @@ _PUBLIC_ enum ndr_err_code ndr_pull_nbt_string(struct ndr_pull *ndr, int ndr_fla > */ > _PUBLIC_ enum ndr_err_code ndr_push_nbt_string(struct ndr_push *ndr, int ndr_flags, const char *s) > { >- if (!(ndr_flags & NDR_SCALARS)) { >- return NDR_ERR_SUCCESS; >- } >- >- while (s && *s) { >- enum ndr_err_code ndr_err; >- char *compname; >- size_t complen; >- uint32_t offset; >- >- /* see if we have pushed the remaining string already, >- * if so we use a label pointer to this string >- */ >- ndr_err = ndr_token_retrieve_cmp_fn(&ndr->nbt_string_list, s, &offset, (comparison_fn_t)strcmp, false); >- if (NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { >- uint8_t b[2]; >- >- if (offset > 0x3FFF) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "offset for nbt string label pointer %u[%08X] > 0x00003FFF", >- offset, offset); >- } >- >- b[0] = 0xC0 | (offset>>8); >- b[1] = (offset & 0xFF); >- >- return ndr_push_bytes(ndr, b, 2); >- } >- >- complen = strcspn(s, "."); >- >- /* we need to make sure the length fits into 6 bytes */ >- if (complen > 0x3F) { >- return ndr_push_error(ndr, NDR_ERR_STRING, >- "component length %u[%08X] > 0x0000003F", >- (unsigned)complen, (unsigned)complen); >- } >- >- if (s[complen] == '.' && s[complen+1] == '\0') { >- complen++; >- } >- >- compname = talloc_asprintf(ndr, "%c%*.*s", >- (unsigned char)complen, >- (unsigned char)complen, >- (unsigned char)complen, s); >- NDR_ERR_HAVE_NO_MEMORY(compname); >- >- /* remember the current componemt + the rest of the string >- * so it can be reused later >- */ >- NDR_CHECK(ndr_token_store(ndr, &ndr->nbt_string_list, s, ndr->offset)); >- >- /* push just this component into the blob */ >- NDR_CHECK(ndr_push_bytes(ndr, (const uint8_t *)compname, complen+1)); >- talloc_free(compname); >- >- s += complen; >- if (*s == '.') s++; >- } >- >- /* if we reach the end of the string and have pushed the last component >- * without using a label pointer, we need to terminate the string >- */ >- return ndr_push_bytes(ndr, (const uint8_t *)"", 1); >+ return ndr_push_dns_string_list(ndr, >+ &ndr->dns_string_list, >+ ndr_flags, >+ s, >+ true); > } > > >diff --git a/librpc/wscript_build b/librpc/wscript_build >index c5f676cd3b6..3ab24da19c7 100644 >--- a/librpc/wscript_build >+++ b/librpc/wscript_build >@@ -415,7 +415,7 @@ bld.SAMBA_SUBSYSTEM('NDR_NBT', > > bld.SAMBA_LIBRARY('ndr_nbt', > source='gen_ndr/ndr_nbt.c ndr/ndr_nbt.c', >- public_deps='ndr NDR_NBT_BUF NDR_SECURITY', >+ public_deps='ndr NDR_NBT_BUF NDR_SECURITY NDR_DNS', > public_headers='gen_ndr/nbt.h gen_ndr/ndr_nbt.h ndr/ndr_nbt.h', > header_path=[ ('gen_ndr*', 'gen_ndr'), ('ndr*', 'ndr')], > pc_files='ndr_nbt.pc', >diff --git a/selftest/knownfail.d/dns_packet b/selftest/knownfail.d/dns_packet >deleted file mode 100644 >index da7347d5af3..00000000000 >--- a/selftest/knownfail.d/dns_packet >+++ /dev/null >@@ -1,2 +0,0 @@ >-samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_half_dotty_components >-samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components >-- >2.17.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Actions:
View
Attachments on
bug 14378
:
15978
|
15982
|
15999
|
16000
|
16001
|
16015
|
16016
|
16034
|
16035
|
16036
|
16037
|
16038
|
16039
|
16040
|
16041
|
16042
|
16045
|
16046
|
16047
|
16048
|
16049
|
16050
|
16051
|
16052
|
16053
|
16057
|
16068
|
16090
|
16093
|
16099