From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Date: Tue, 19 May 2020 22:05:16 +0000 (+1200)
Subject: ndr: pull_dns_string: don't allow dots or '\0' in labels
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2028b2204ac1620284ffb254f76b28fe900e9041;p=thirdparty%2Fsamba.git

ndr: pull_dns_string: don't allow dots or '\0' in labels

We use a copy function that returns false if the copied string
contains the bad characters, and true otherwise.

As a special case, we allow a '.' as the last character, because an
NBT name with a trailing dot is sometimes used as a username, and we
need to match these exactly, even though the dotless form is
semantically the same (per RFC).

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14378

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andreas Schneider <asn@samba.org>
---

diff --git a/librpc/ndr/ndr_dns_utils.c b/librpc/ndr/ndr_dns_utils.c
index befb106772b..9b603b364f1 100644
--- a/librpc/ndr/ndr_dns_utils.c
+++ b/librpc/ndr/ndr_dns_utils.c
@@ -8,6 +8,37 @@
  */
 #define MAX_COMPONENTS 128
 
+
+
+/*
+ * Copy a DNS component. If it is "good" as a DNS component, return true,
+ * otherwise return false. In either case the whole copy is performed.
+ */
+static bool dns_component_copy(uint8_t *dest, uint8_t *src, size_t len)
+{
+	size_t i;
+	bool ret = true;
+	for (i = 0; i < len; i++) {
+		uint8_t c = src[i];
+		dest[i] = c;
+		if (c == '\0') {
+			ret = false;
+		} else if (c == '.' && i != len - 1) {
+			/*
+			 * As a special case, for NBT addresses that are also
+			 * usernames, we allow a dot as the last character.
+			 *
+			 * This will not be treated symmetrically on the push
+			 * side unless the component is the last component of
+			 * an NBT name. In other cases (DNS, non-final NBT)
+			 * the trailing dot would truncate the address on push.
+			 */
+			ret = false;
+		}
+	}
+	return ret;
+}
+
 /*
   pull one component of a dns/nbt string
 */
@@ -20,6 +51,7 @@ static enum ndr_err_code ndr_pull_component(struct ndr_pull *ndr,
 {
 	uint8_t len;
 	unsigned int loops = 0;
+	bool comp_is_valid;
 	*component_len = 0;
 	while (loops < 5) {
 		if (*offset >= ndr->data_size) {
@@ -61,7 +93,16 @@ static enum ndr_err_code ndr_pull_component(struct ndr_pull *ndr,
 					      "length too long",
 					      err_name);
 		}
-		memcpy(component, &ndr->data[1 + *offset], len);
+
+		comp_is_valid = dns_component_copy(component,
+						   &ndr->data[1 + *offset],
+						   len);
+		if (! comp_is_valid) {
+			return ndr_pull_error(ndr, NDR_ERR_STRING,
+					      "BAD %s NAME component, "\
+					      "undesirable characters",
+					      err_name);
+		}
 		*component_len = len;
 		*offset += len + 1;
 		*max_offset = MAX(*max_offset, *offset);