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

@@ -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_push_dns_string_list(ndr,
-		return NDR_ERR_SUCCESS;
+					&ndr->dns_string_list,
-	}
+					ndr_flags,
-
+					s);
-	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);
 }
 
 _PUBLIC_ enum ndr_err_code ndr_pull_dns_txt_record(struct ndr_pull *ndr, int ndr_flags, struct dns_txt_record *r)

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);
+}

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);

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'
     )