From: tmarkwalder <tmark@isc.org>
Date: Wed, 3 May 2017 12:42:22 +0000 (-0400)
Subject: [master] v6 FQDN option unpacking now handles values with spaces and non-printables
X-Git-Tag: v4_4_0b1_f1~103
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2f5fefd3a806d9f4387200b0e0bf6dd9a6b9f85b;p=thirdparty%2Fdhcp.git

[master] v6 FQDN option unpacking now handles values with spaces and non-printables
    Merged in rt43592.
---

diff --git a/RELNOTES b/RELNOTES
index 12f34481f..cdda8cd9e 100644
--- a/RELNOTES
+++ b/RELNOTES
@@ -172,6 +172,13 @@ by Eric Young (eay@cryptsoft.com).
   reporting the issue.
   [ISC-Bugs #28038]
 
+- DHCP6 FQDN option unpacking code now correctly handles values that contain
+  spaces, special, or non-printable characters.  Prior to this the buffer
+  size needed was underestitmate causing a conversion error message to
+  be logged and DNS updates to be skipped. Thanks to Fernando Soto at
+  BlueCat Networks for bringing the matter to our attention.
+  [ISC-Bugs #43592]
+
 			Changes since 4.3.0 (bug fixes)
 
 - Tidy up several small tickets.
diff --git a/common/options.c b/common/options.c
index 223726ca0..b0dcab426 100644
--- a/common/options.c
+++ b/common/options.c
@@ -3594,12 +3594,17 @@ fqdn6_universe_decode(struct option_state *options,
 		return 0;
 
 	/* Save the contents of the option in a buffer.  There are 3
-	 * one-byte values we record from the packet, so we go ahead
-	 * and allocate a bigger buffer to accommodate them.  But the
-	 * 'length' we got (because it is a DNS encoded string) is
-	 * one longer than we need...so we only add two extra octets.
-	 */
-	if (!buffer_allocate(&bp, length + 2, MDL)) {
+	 * one-byte values we record from the packet. The input is
+	 * DNS encoded and to be safe we'll assume that each character
+	 * is non-printable and will be converted to an escaped number:
+	 * "\\nnn".  Yes, we'll have dead space pretty much all the time
+	 * but the alternative is to basically dry run the conversion
+	 * first to calculate the precise size or reallocate to a smaller
+	 * buffer later, either of which is a bigger performance hit than
+	 * just doing a generous allocation. */
+	unsigned bp_size = 3 + (length * 4);
+
+	if (!buffer_allocate(&bp, bp_size, MDL)) {
 		log_error("No memory for dhcp6.fqdn option buffer.");
 		return 0;
 	}
@@ -3611,7 +3616,6 @@ fqdn6_universe_decode(struct option_state *options,
 		goto error;
 
 	/* XXX: We need to process 'The "N" bit'. */
-
 	if (buffer[0] & 1) /* server-update. */
 		bp->data[2] = 1;
 	else
@@ -3631,7 +3635,7 @@ fqdn6_universe_decode(struct option_state *options,
 		goto error;
 
 	/* Convert the domain name to textual representation for config. */
-	len = MRns_name_ntop(buffer + 1, (char *)bp->data + 3, length - 1);
+	len = MRns_name_ntop(buffer + 1, (char *)bp->data + 3, bp_size - 3);
 	if (len == -1) {
 		log_error("Unable to convert dhcp6.fqdn domain name to "
 			  "printable form.");