Ported domain name fix2-domain-name-appears-to-be-wrongly-encoded-v4-1-esv

6 files changed, 169 insertions, 14 deletions

diff --git a/RELNOTES b/RELNOTES
index e1561f4f..1b3ba220 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -82,6 +82,11 @@ dhcp-users@lists.isc.org.
   reporting the issue.
   [ISC-Bugs #19]
 
+- The "d" domain name option format was incorrectly handled as text
+  instead of RFC 1035 wire format. Thanks to Jay Doran at BlueCat Networks
+  for reporting this issue.
+  [Gitlab #2]
+
 			Changes since 4.1-ESV-R15
 
 - Corrected dhclient command line parsing for --dad-wait-time that causes
diff --git a/common/options.c b/common/options.c
index 5b4f17d9..09723c60 100644
--- a/common/options.c
+++ b/common/options.c
@@ -35,6 +35,8 @@ struct option *vendor_cfg_option;
 
 static int pretty_text(char **, char *, const unsigned char **,
 			 const unsigned char *, int);
+static int pretty_dname(char **, char *, const unsigned char *,
+			 const unsigned char *);
 static int pretty_domain(char **, char *, const unsigned char **,
 			 const unsigned char *);
 static int prepare_option_buffer(struct universe *universe, struct buffer *bp,
@@ -1601,7 +1603,6 @@ format_has_text(format)
 	p = format;
 	while (*p != '\0') {
 		switch (*p++) {
-		    case 'd':
 		    case 't':
 			return 1;
 
@@ -1615,6 +1616,7 @@ format_has_text(format)
 		    case 'X':
 		    case 'x':
 		    case 'D':
+		    case 'd':
 			return 0;
 
 		    case 'c':
@@ -1857,8 +1859,23 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
 			numhunk = -2;
 			break;
 		      case 'd':
-			fmtbuf[l] = 't';
-			/* Fall Through ! */
+			/* Should not be optional, array or compressed */
+			if ((option->format[i+1] == 'o') ||
+			    (option->format[i+1] == 'a') ||
+			    (option->format[i+1] == 'A') ||
+			    (option->format[i+1] == 'c')) {
+				log_error("%s: Illegal use of domain name: %s",
+					  option->name,
+					  &(option->format[i-1]));
+				fmtbuf[l + 1] = 0;
+			}
+			k = MRns_name_len(data + len, data + hunksize);
+			if (k == -1) {
+				log_error("Invalid domain name.");
+				return "<error>";
+			}
+			hunksize += k;
+			break;
 		      case 't':
 			fmtbuf[l + 1] = 0;
 			numhunk = -2;
@@ -2012,6 +2029,18 @@ const char *pretty_print_option (option, data, len, emit_commas, emit_quotes)
 				}
 				*op = 0;
 				break;
+			      case 'd': /* RFC1035 format name */
+				k = MRns_name_len(data + len, dp);
+				/* Already tested... */
+				if (k == -1) {
+					log_error("invalid domain name.");
+					return "<error>";
+				}
+				pretty_dname(&op, endbuf-1, dp, data + len);
+				/* pretty_dname does not add the nul */
+				*op = '\0';
+				dp += k;
+				break;
 			      case 'D': /* RFC1035 format name list */
 				for( ; dp < (data + len) ; dp += k) {
 					unsigned char nbuff[NS_MAXCDNAME];
@@ -4184,6 +4213,56 @@ pretty_text(char **dst, char *dend, const unsigned char **src,
 }
 
 static int
+pretty_dname(char **dst, char *dend, const unsigned char *src,
+	     const unsigned char *send)
+{
+	const unsigned char *tend;
+	const unsigned char *srcp = src;
+	int count = 0;
+	int tsiz, status;
+
+	if (dst == NULL || dend == NULL || src == NULL || send == NULL ||
+	    *dst == NULL || ((*dst + 1) > dend) || (src >= send))
+		return -1;
+
+	do {
+		/* Continue loop until end of src buffer. */
+		if (srcp >= send)
+			break;
+
+		/* Consume tag size. */
+		tsiz = *srcp;
+		srcp++;
+
+		/* At root, finis. */
+		if (tsiz == 0)
+			break;
+
+		tend = srcp + tsiz;
+
+		/* If the tag exceeds the source buffer, it's illegal.
+		 * This should also trap compression pointers (which should
+		 * not be in these buffers).
+		 */
+		if (tend > send)
+			return -1;
+
+		/* dend-1 leaves room for a trailing dot and quote. */
+		status = pretty_escape(dst, dend-1, &srcp, tend);
+
+		if ((status == -1) || ((*dst + 1) > dend))
+			return -1;
+
+		**dst = '.';
+		(*dst)++;
+		count += status + 1;
+	}
+	while(1);
+
+	return count;
+}
+
+static int
 pretty_domain(char **dst, char *dend, const unsigned char **src,
 	      const unsigned char *send)
 {
diff --git a/common/parse.c b/common/parse.c
index ff31e28c..eabf3ed2 100644
--- a/common/parse.c
+++ b/common/parse.c
@@ -5266,15 +5266,13 @@ int parse_option_token (rv, cfile, fmt, expr, uniform, lookups)
 		break;
 
 	      case 'd': /* Domain name... */
-		val = parse_host_name (cfile);
-		if (!val) {
-			parse_warn (cfile, "not a valid domain name.");
-			skip_to_semi (cfile);
+		t = parse_domain_name(cfile);
+		if (!t) {
+			parse_warn(cfile, "not a valid domain name.");
+			skip_to_semi(cfile);
 			return 0;
 		}
-		len = strlen (val);
-		freeval = ISC_TRUE;
-		goto make_string;
+		break;
 
 	      case 't': /* Text string... */
 		token = next_token (&val, &len, cfile);
@@ -5286,7 +5284,6 @@ int parse_option_token (rv, cfile, fmt, expr, uniform, lookups)
 			}
 			return 0;
 		}
-	      make_string:
 		if (!make_const_data (&t, (const unsigned char *)val,
 				      len, 1, 1, MDL))
 			log_fatal ("No memory for concatenation");
@@ -5938,3 +5935,42 @@ parse_domain_list(struct parse *cfile, int compress)
 	return t;
 }
 
+struct expression *
+parse_domain_name(struct parse *cfile)
+{
+	const char *val;
+	struct expression *t = NULL;
+	unsigned len;
+	int result;
+	unsigned char buf[NS_MAXCDNAME];
+
+	val = parse_host_name(cfile);
+	if (!val) {
+		return NULL;
+	}
+	result = MRns_name_pton(val, buf, sizeof(buf));
+	/* No longer need val */
+	dfree((char *)val, MDL);
+
+	/* result == 1 means the input was fully qualified.
+	 * result == 0 means the input wasn't.
+	 * result == -1 means bad things.
+	 */
+	if (result < 0) {
+		parse_warn(cfile, "Error assembling domain name: %m");
+		return NULL;
+	}
+
+	/* Compute the used length */
+	len = 0;
+	while (buf[len] != 0) {
+		len += buf[len] + 1;
+	}
+	/* Count the last label (0). */
+	len++;
+
+	if (!make_const_data(&t, buf, len, 1, 1, MDL))
+		log_fatal("No memory for domain name object.");
+
+	return t;
+}
diff --git a/includes/dhcpd.h b/includes/dhcpd.h
index 6643dc4d..0c77c1c8 100644
--- a/includes/dhcpd.h
+++ b/includes/dhcpd.h
@@ -1862,7 +1862,7 @@ int parse_auth_key (struct data_string *, struct parse *);
 int parse_warn (struct parse *, const char *, ...)
 	__attribute__((__format__(__printf__,2,3)));
 struct expression *parse_domain_list(struct parse *cfile, int);
-
+struct expression *parse_domain_name(struct parse *cfile);
 
 /* tree.c */
 #if defined (NSUPDATE)
diff --git a/includes/minires/minires.h b/includes/minires/minires.h
index 6fc0d328..486faaba 100644
--- a/includes/minires/minires.h
+++ b/includes/minires/minires.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004,2007-2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2019 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 2001-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -47,6 +47,7 @@ isc_result_t minires_nupdate (res_state, ns_updrec *);
 int minires_ninit (res_state);
 ns_rcode isc_rcode_to_ns (isc_result_t);
 
+int MRns_name_len(const unsigned char *, const unsigned char *);
 int MRns_name_compress(const char *, u_char *, size_t, const unsigned char **,
 		       const unsigned char **);
 int MRns_name_unpack(const unsigned char *, const unsigned char *,
@@ -114,6 +115,7 @@ int MRns_name_compress_list(const char*, int buflen, unsigned char*, size_t);
 #define ns_name_pton MRns_name_pton
 #define ns_name_unpack MRns_name_unpack
 #define ns_name_pack MRns_name_pack
+#define ns_name_len MRns_name_len
 #define ns_name_compress MRns_name_compress
 #define ns_name_skip MRns_name_skip
 #define ns_subdomain MRns_subdomain
diff --git a/minires/ns_name.c b/minires/ns_name.c
index d7562c42..de2f7a62 100644
--- a/minires/ns_name.c
+++ b/minires/ns_name.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2004,2009,2014 by Internet Systems Consortium, Inc. ("ISC")
+ * Copyright (c) 2004-2019 by Internet Systems Consortium, Inc. ("ISC")
  * Copyright (c) 1996-2003 by Internet Software Consortium
  *
  * Permission to use, copy, modify, and distribute this software for any
@@ -48,6 +48,39 @@ static int		dn_find(const u_char *, const u_char *,
 /* Public. */
 
 /*
+ * ns_name_len(eom, src)
+ *	Compute the length of encoded uncompressed domain name.
+ * return:
+ *	-1 if it fails, or to be consumed octets if it succeeds.
+ */
+int
+ns_name_len(const u_char *eom, const u_char *src)
+{
+	const u_char *srcp;
+	unsigned n;
+	int len;
+
+	len = -1;
+	srcp = src;
+	if (srcp >= eom) {
+		errno = EMSGSIZE;
+		return (-1);
+	}
+	/* Fetch next label in domain name. */
+	while ((n = *srcp++) != 0) {
+		/* Limit checks. */
+		if (srcp + n >= eom) {
+			errno = EMSGSIZE;
+			return (-1);
+		}
+		srcp += n;
+	}
+	if (len < 0)
+		len = srcp - src;
+	return (len);
+}
+
+/*
  * ns_name_ntop(src, dst, dstsiz)
  *	Convert an encoded domain name to printable ascii as per RFC1035.
  * return: