CVE-2020-10745: ndr/dns-utils: prepare for NBT compatibility
authorDouglas Bagnall <douglas.bagnall@catalyst.net.nz>
Thu, 14 May 2020 22:52:45 +0000 (10:52 +1200)
committerKarolin Seeger <kseeger@samba.org>
Thu, 2 Jul 2020 09:01:41 +0000 (09:01 +0000)
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
librpc/ndr/ndr_dns_utils.c
librpc/ndr/ndr_dns_utils.h
librpc/ndr/ndr_nbt.c
librpc/wscript_build
selftest/knownfail.d/dns_packet
selftest/knownfail.d/ndr_dns_nbt [deleted file]

index 68a3c9de78200130eb6a91ef8ac244e8708f7b3f..966e0b5978647892966088dc48b091d185dc462c 100644 (file)
@@ -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)
index b7f11dbab4e967d892a0ff200dbdd978f1306fb5..325d9c68bea5c448b8f79b4228fe908465d6ad0d 100644 (file)
@@ -9,9 +9,32 @@
 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;
+       size_t max_length;
+       if (is_nbt) {
+               use_compression = true;
+               /*
+                * Max length is longer in NBT/Wins, because Windows counts
+                * the semi-decompressed size of the netbios name (16 bytes)
+                * rather than the wire size of 32, which is what you'd expect
+                * if it followed RFC1002 (it uses the short form in
+                * [MS-WINSRA]). In other words the maximum size of the
+                * "scope" is 237, not 221.
+                *
+                * We make the size limit slightly larger than 255 + 16,
+                * because the 237 scope limit is already enforced in the
+                * winsserver code with a specific return value; bailing out
+                * here would muck with that.
+                */
+               max_length = 274;
+       } else {
+               use_compression = !(ndr->flags & LIBNDR_FLAG_NO_COMPRESSION);
+               max_length = 255;
+       }
 
        if (!(ndr_flags & NDR_SCALARS)) {
                return NDR_ERR_SUCCESS;
@@ -23,7 +46,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 +89,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 +106,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));
                }
@@ -89,9 +120,10 @@ enum ndr_err_code ndr_push_dns_string_list(struct ndr_push *ndr,
                if (*s == '.') {
                        s++;
                }
-               if (s - start > 255) {
+               if (s - start > max_length) {
                        return ndr_push_error(ndr, NDR_ERR_STRING,
-                                             "name > 255 character long");
+                                             "name > %zu character long",
+                                             max_length);
                }
        }
 
index 823e32011127d1589fbddea4b9edf07d7d0d509a..71a65433bbb1d8fbdf180694eef3b70d86061ebd 100644 (file)
@@ -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);
index b15dc5c06e5229f4f9d66c5cf43caac7ba3a2141..8ed9f0a5f05489288fe912cb7c7ae51991bcf5c4 100644 (file)
@@ -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);
 }
 
 
index 8fa0dd640303173d00b08d12c582a2f25022474c..398b299f3393d81a22f3c7f26b2c7ca0651657f8 100644 (file)
@@ -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',
@@ -718,6 +718,5 @@ bld.SAMBA_BINARY('test_ndr_dns_nbt',
                       cmocka
                       ndr
                       ndr_nbt
-                      NDR_DNS
                       ''',
                  for_selftest=True)
index 0662266f689c4b088e7316cfda637e3187351a17..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 100644 (file)
@@ -1 +0,0 @@
-samba.tests.dns_packet.samba.tests.dns_packet.TestNbtPackets.test_127_very_dotty_components
diff --git a/selftest/knownfail.d/ndr_dns_nbt b/selftest/knownfail.d/ndr_dns_nbt
deleted file mode 100644 (file)
index 603395c..0000000
+++ /dev/null
@@ -1,2 +0,0 @@
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_all_dots
-librpc.ndr.ndr_dns_nbt.test_ndr_nbt_string_half_dots