[SCM] Samba Shared Repository - branch master updated

Karolin Seeger kseeger at samba.org
Thu Jul 2 10:27:05 UTC 2020


The branch, master has been updated
       via  3cc0f1eeda5 CVE-2020-14303: s4 nbt: fix busy loop on empty UDP packet
       via  b232a7bc546 CVE-2020-14303 Ensure an empty packet will not DoS the NBT server
       via  17fc8d2bfb2 CVE-2020-10760 dsdb: Add tests for paged_results and VLV over the Global Catalog port
       via  32c333def9a CVE-2020-10760 dsdb: Ensure a proper talloc tree for saved controls
       via  cc3a67760cf CVE-2020-10745: ndr/dns-utils: prepare for NBT compatibility
       via  c3fa8ada43e CVE-2020-10745: dns_util/push: forbid names longer than 255 bytes
       via  51a4571849c CVE-2020-10745: ndr_dns: do not allow consecutive dots
       via  bb637379056 CVE-2020-10745: ndr/dns_utils: correct a comment
       via  601e8a3f690 CVE-2020-10745: ndr_dns: move ndr_push_dns_string core into sharable function
       via  bc896d75295 CVE-2020-10745: librpc/tests: cmocka tests of dns and ndr strings
       via  f4b2fd00fe3 CVE-2020-10745: pytests: hand-rolled invalid dns/nbt packet tests
       via  d8b9bb274b7 CVE-2020-10730: lib ldb: Check if ldb_lock_backend_callback called twice
       via  f88b69f5430 CVE-2020-10730: s4 dsdb vlv_pagination: Prevent repeat call of ldb_module_done
       via  4d99cab6172 CVE-2020-10730: s4 dsdb paged_results: Prevent repeat call of ldb_module_done
       via  9197d85c52d CVE-2020-10730: dsdb: Ban the combination of paged_results and VLV
       via  60078f4f6fd CVE-2020-10730: dsdb: Fix crash when vlv and paged_results are combined
       via  cac84d9161f CVE-2020-10730: selftest: Add test to show that VLV and paged_results are incompatible
       via  ff0b1df8569 CVE-2020-10730: vlv: Another workaround for mixing ASQ and VLV
       via  ec1c7d6208d CVE-2020-10730: selftest: Add test to confirm VLV interaction with ASQ
       via  914e0cfa085 CVE-2020-10730: vlv: Do not re-ASQ search the results of an ASQ search with VLV
       via  b2f27a47b33 CVE-2020-10730: vlv: Use strcmp(), not strncmp() checking the NULL terminated control OIDs
      from  f59490dc2d0 s3: libsmb: Fix SMB2 client rename bug to a Windows server.

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit 3cc0f1eeda5f133532dda31eef9fc1b394127e50
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Wed Jun 24 14:27:08 2020 +1200

    CVE-2020-14303: s4 nbt: fix busy loop on empty UDP packet
    
    An empty UDP packet put the nbt server into a busy loop that consumes
    100% of a cpu.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14417
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    
    Autobuild-User(master): Karolin Seeger <kseeger at samba.org>
    Autobuild-Date(master): Thu Jul  2 10:26:24 UTC 2020 on sn-devel-184

commit b232a7bc546f8e6fdb638164a8411772e67c8864
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Thu Jun 25 11:59:54 2020 +1200

    CVE-2020-14303 Ensure an empty packet will not DoS the NBT server
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 17fc8d2bfb21294667507fda8f3a7160640e2da0
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Mon Jun 8 16:32:14 2020 +1200

    CVE-2020-10760 dsdb: Add tests for paged_results and VLV over the Global Catalog port
    
    This should avoid a regression.
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit 32c333def9ad5a1c67abee320cf5f3c4f2cb1e5c
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Fri Jun 5 22:14:48 2020 +1200

    CVE-2020-10760 dsdb: Ensure a proper talloc tree for saved controls
    
    Otherwise a paged search on the GC port will fail as the ->data was
    not kept around for the second page of searches.
    
    An example command to produce this is
     bin/ldbsearch --paged -H ldap://$SERVER:3268 -U$USERNAME%$PASSWORD
    
    This shows up later in the partition module as:
    
    ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00151ef20 at pc 0x7fec3f801aac bp 0x7ffe8472c270 sp 0x7ffe8472c260
    READ of size 4 at 0x60b00151ef20 thread T0 (ldap(0))
        #0 0x7fec3f801aab in talloc_chunk_from_ptr ../../lib/talloc/talloc.c:526
        #1 0x7fec3f801aab in __talloc_get_name ../../lib/talloc/talloc.c:1559
        #2 0x7fec3f801aab in talloc_check_name ../../lib/talloc/talloc.c:1582
        #3 0x7fec1b86b2e1 in partition_search ../../source4/dsdb/samdb/ldb_modules/partition.c:780
    
    or
    
    smb_panic_default: PANIC (pid 13287): Bad talloc magic value - unknown value
    (from source4/dsdb/samdb/ldb_modules/partition.c:780)
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14402
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>

commit cc3a67760cf9faaad3af73b1eed9e2ef85276633
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri May 15 10:52:45 2020 +1200

    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 at catalyst.net.nz>

commit c3fa8ada43e5383f8dec6cd93cbc21139df704f7
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri May 15 00:06:08 2020 +1200

    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 at catalyst.net.nz>

commit 51a4571849c5a84b994ce72908eac8141c2d72ed
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Apr 25 11:10:18 2020 +1200

    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 at catalyst.net.nz>

commit bb6373790567ed56a56ea968cfee8da2f92e5cc6
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Apr 25 11:03:30 2020 +1200

    CVE-2020-10745: ndr/dns_utils: correct a comment
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

commit 601e8a3f690bbf5b9758cb148e9425df2993a2b0
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Sat Apr 25 11:02:08 2020 +1200

    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 at catalyst.net.nz>

commit bc896d75295870c410709dc590364bd9916240cf
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Fri Jun 12 14:26:38 2020 +1200

    CVE-2020-10745: librpc/tests: cmocka tests of dns and ndr strings
    
    These time the push and pull function in isolation.
    
    Timing should be under 0.0001 seconds on even quite old hardware; we
    assert it must be under 0.2 seconds.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378
    
    Signed-off-by: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>

commit f4b2fd00fe34e3e8d934da3a11329f3d79b437d9
Author: Douglas Bagnall <douglas.bagnall at catalyst.net.nz>
Date:   Thu Jun 11 17:38:51 2020 +1200

    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 at catalyst.net.nz>

commit d8b9bb274b7e7a390cf3bda9cd732cb2227bdbde
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Wed May 13 10:56:56 2020 +1200

    CVE-2020-10730: lib ldb: Check if ldb_lock_backend_callback called twice
    
    Prevent use after free issues if ldb_lock_backend_callback is called
    twice, usually due to ldb_module_done being called twice. This can happen if a
    module ignores the return value from function a function that calls
    ldb_module_done as part of it's error handling.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit f88b69f543002a05c8fb18ce21a16fad8aec5063
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Mon May 18 12:37:39 2020 +1200

    CVE-2020-10730: s4 dsdb vlv_pagination: Prevent repeat call of ldb_module_done
    
    Check the return code from vlv_results, if it is not LDB_SUCCESS
    ldb_module_done has already been called, and SHOULD NOT be called again.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 4d99cab6172a515026275a1ed19ded4def8710d7
Author: Gary Lockyer <gary at catalyst.net.nz>
Date:   Mon May 18 12:36:57 2020 +1200

    CVE-2020-10730: s4 dsdb paged_results: Prevent repeat call of ldb_module_done
    
    Check the return code from paged_results, if it is not LDB_SUCCESS
    ldb_module_done has already been called, and SHOULD NOT be called again.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Gary Lockyer <gary at catalyst.net.nz>
    Reviewed-by: Andrew Bartlett <abartlet at samba.org>

commit 9197d85c52dc07b7e66eebb7da21e0fcd40a21c9
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 6 16:18:19 2020 +1200

    CVE-2020-10730: dsdb: Ban the combination of paged_results and VLV
    
    This (two different paging controls) makes no sense and fails against
    Windows Server 1709.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 60078f4f6fde5daeadbaf1662e56564088cf7d6a
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 6 17:05:30 2020 +1200

    CVE-2020-10730: dsdb: Fix crash when vlv and paged_results are combined
    
    The GUID is not returned in the DN for some reason in this (to be banned)
    combination.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit cac84d9161f8d0d772961fed60410e608e260d61
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Wed May 6 16:19:01 2020 +1200

    CVE-2020-10730: selftest: Add test to show that VLV and paged_results are incompatible
    
    As tested against Windows Server 1709
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit ff0b1df8569a3f066e4061ea7ddb450120e3b063
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 5 16:34:11 2020 +1200

    CVE-2020-10730: vlv: Another workaround for mixing ASQ and VLV
    
    This is essentially an alternative patch, but without the correct
    behaviour.  Instead this just avoids a segfault.
    
    Included in case we have something simialr again in
    another module.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit ec1c7d6208d434bd9c91f288259e903b5b3a0f52
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 5 13:16:48 2020 +1200

    CVE-2020-10730: selftest: Add test to confirm VLV interaction with ASQ
    
    Tested against Windows 1709.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit 914e0cfa0857378cbe21fc5752302ae572e8162e
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 5 12:55:57 2020 +1200

    CVE-2020-10730: vlv: Do not re-ASQ search the results of an ASQ search with VLV
    
    This is a silly combination, but at least try and keep the results sensible
    and avoid a double-dereference.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

commit b2f27a47b33e06fbcf8470b53f598bd610e35296
Author: Andrew Bartlett <abartlet at samba.org>
Date:   Tue May 5 12:54:59 2020 +1200

    CVE-2020-10730: vlv: Use strcmp(), not strncmp() checking the NULL terminated control OIDs
    
    The end result is the same, as sizeof() includes the trailing NUL, but this
    avoids having to think about that.
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=14364
    
    Signed-off-by: Andrew Bartlett <abartlet at samba.org>
    Reviewed-by: Gary Lockyer <gary at catalyst.net.nz>

-----------------------------------------------------------------------

Summary of changes:
 lib/ldb/common/ldb.c                               |   9 +-
 libcli/nbt/nbtsocket.c                             |  17 +-
 librpc/ndr/ndr_dns.c                               |  80 +------
 librpc/ndr/ndr_dns_utils.c                         | 134 ++++++++++++
 librpc/ndr/ndr_dns_utils.h                         |   6 +
 librpc/ndr/ndr_nbt.c                               |  72 +------
 librpc/tests/test_ndr_dns_nbt.c                    | 236 +++++++++++++++++++++
 librpc/wscript_build                               |  13 +-
 python/samba/tests/dns_packet.py                   | 230 ++++++++++++++++++++
 .../__init__.py => selftest/knownfail.d/dns_packet |   0
 selftest/knownfail.d/vlv                           |   2 +-
 source4/dsdb/samdb/ldb_modules/paged_results.c     |  65 +++++-
 source4/dsdb/samdb/ldb_modules/vlv_pagination.c    | 102 +++++++--
 source4/dsdb/tests/python/asq.py                   |  54 +++++
 source4/dsdb/tests/python/vlv.py                   | 184 ++++++++++------
 source4/selftest/tests.py                          |  12 ++
 16 files changed, 985 insertions(+), 231 deletions(-)
 create mode 100644 librpc/ndr/ndr_dns_utils.c
 create mode 100644 librpc/ndr/ndr_dns_utils.h
 create mode 100644 librpc/tests/test_ndr_dns_nbt.c
 create mode 100644 python/samba/tests/dns_packet.py
 copy buildtools/wafsamba/__init__.py => selftest/knownfail.d/dns_packet (100%)


Changeset truncated at 500 lines:

diff --git a/lib/ldb/common/ldb.c b/lib/ldb/common/ldb.c
index 8c86dca45a1..0fec89a52a8 100644
--- a/lib/ldb/common/ldb.c
+++ b/lib/ldb/common/ldb.c
@@ -1018,6 +1018,13 @@ static int ldb_lock_backend_callback(struct ldb_request *req,
 	struct ldb_db_lock_context *lock_context;
 	int ret;
 
+	if (req->context == NULL) {
+		/*
+		 * The usual way to get here is to ignore the return codes
+		 * and continuing processing after an error.
+		 */
+		abort();
+	}
 	lock_context = talloc_get_type(req->context,
 				       struct ldb_db_lock_context);
 
@@ -1032,7 +1039,7 @@ static int ldb_lock_backend_callback(struct ldb_request *req,
 		 * If this is a LDB_REPLY_DONE or an error, unlock the
 		 * DB by calling the destructor on this context
 		 */
-		talloc_free(lock_context);
+		TALLOC_FREE(req->context);
 		return ret;
 	}
 
diff --git a/libcli/nbt/nbtsocket.c b/libcli/nbt/nbtsocket.c
index f682b233fd1..97b0ca34337 100644
--- a/libcli/nbt/nbtsocket.c
+++ b/libcli/nbt/nbtsocket.c
@@ -167,8 +167,23 @@ static void nbt_name_socket_recv(struct nbt_name_socket *nbtsock)
 		return;
 	}
 
+	/*
+	 * Given a zero length, data_blob_talloc() returns the
+	 * NULL blob {NULL, 0}.
+	 *
+	 * We only want to error return here on a real out of memory condition
+	 * (i.e. dsize != 0, so the UDP packet has data, but the return of the
+	 * allocation failed, so blob.data==NULL).
+	 *
+	 * Given an actual zero length UDP packet having blob.data == NULL
+	 * isn't an out of memory error condition, that's the defined semantics
+	 * of data_blob_talloc() when asked for zero bytes.
+	 *
+	 * We still need to continue to do the zero-length socket_recvfrom()
+	 * read in order to clear the "read pending" condition on the socket.
+	 */
 	blob = data_blob_talloc(tmp_ctx, NULL, dsize);
-	if (blob.data == NULL) {
+	if (blob.data == NULL && dsize != 0) {
 		talloc_free(tmp_ctx);
 		return;
 	}
diff --git a/librpc/ndr/ndr_dns.c b/librpc/ndr/ndr_dns.c
index d37c8cc2ece..966e0b59786 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,11 @@ _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,
+					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
new file mode 100644
index 00000000000..325d9c68bea
--- /dev/null
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -0,0 +1,134 @@
+#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,
+					   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;
+	}
+
+	while (s && *s) {
+		enum ndr_err_code ndr_err;
+		char *compname;
+		size_t complen;
+		uint32_t offset;
+
+		if (use_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, ".");
+
+		/* the length must fit into 6 bits (i.e. <= 63) */
+		if (complen > 0x3F) {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "component length %u[%08X] > " \
+					      "0x0000003F",
+					      (unsigned)complen,
+					      (unsigned)complen);
+		}
+
+		if (complen == 0 && s[complen] == '.') {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "component length is 0 "
+					      "(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,
+						(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 (use_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 (s - start > max_length) {
+			return ndr_push_error(ndr, NDR_ERR_STRING,
+					      "name > %zu character long",
+					      max_length);
+		}
+	}
+
+	/* 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..71a65433bbb
--- /dev/null
+++ b/librpc/ndr/ndr_dns_utils.h
@@ -0,0 +1,6 @@
+
+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,
+					   bool is_nbt);
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/tests/test_ndr_dns_nbt.c b/librpc/tests/test_ndr_dns_nbt.c
new file mode 100644
index 00000000000..1e2ef45c10d
--- /dev/null
+++ b/librpc/tests/test_ndr_dns_nbt.c
@@ -0,0 +1,236 @@
+/*
+ * Tests for librpc ndr functions
+ *
+ * Copyright (C) Catalyst.NET Ltd 2020
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+#include "replace.h"
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include "includes.h"
+#include "librpc/ndr/libndr.h"
+#include "librpc/gen_ndr/ndr_dns.h"
+#include "librpc/gen_ndr/ndr_nbt.h"
+#include "lib/util/time.h"
+
+#define NBT_NAME "EOGFGLGPCACACACACACACACACACACACA" /* "neko" */
+
+
+static DATA_BLOB generate_obnoxious_dns_name(TALLOC_CTX *mem_ctx,
+					     size_t n_labels,
+					     size_t dot_every,
+					     bool is_nbt)
+{
+	size_t i, j;
+	char *s;
+	DATA_BLOB name = data_blob_talloc(mem_ctx, NULL, 64 * n_labels + 1);
+	assert_non_null(name.data);
+
+	s = (char*)name.data;
+	if (is_nbt) {
+		size_t len = strlen(NBT_NAME);
+		*s = len;
+		s++;
+		memcpy(s, NBT_NAME, len);
+		s += len;
+		n_labels--;
+	}
+
+	for (i = 0; i < n_labels; i++) {
+		*s = 63;
+		s++;
+		for (j = 0; j < 63; j++) {
+			if (j % dot_every == (dot_every - 1)) {
+				*s = '.';
+			} else {
+				*s = 'x';
+			}
+			s++;
+		}
+	}
+	*s = 0;
+	s++;
+	name.length = s - (char*)name.data;
+	return name;
+}
+
+
+static char *_test_ndr_pull_dns_string_list(TALLOC_CTX *mem_ctx,
+					    size_t n_labels,
+					    size_t dot_every,
+					    bool is_nbt)
+{
+	enum ndr_err_code ndr_err;
+	DATA_BLOB blob = generate_obnoxious_dns_name(mem_ctx,
+						     n_labels,
+						     dot_every,
+						     is_nbt);
+
+	char *name;
+	ndr_pull_flags_fn_t fn;
+
+	if (is_nbt) {
+		fn = (ndr_pull_flags_fn_t)ndr_pull_nbt_string;
+	} else {
+		fn = (ndr_pull_flags_fn_t)ndr_pull_dns_string;
+	}
+
+	ndr_err = ndr_pull_struct_blob(&blob,
+				       mem_ctx,
+				       &name,
+				       fn);
+	/* Success here is not expected, but we let it go to measure timing. */
+	if (ndr_err == NDR_ERR_SUCCESS) {


-- 
Samba Shared Repository



More information about the samba-cvs mailing list