From: Roy Marples <roy@marples.name>
Date: Mon, 4 Jan 2016 17:43:28 +0000 (+0000)
Subject: dhcp_optlen now returns the length of the data we can sanely work on
X-Git-Tag: v6.10.0~14
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1475a702df74b120db847991bc011e3441a045b8;p=thirdparty%2Fdhcpcd.git

dhcp_optlen now returns the length of the data we can sanely work on
given the option definition and data length.
Call dhcp_optlen in dhcp_envoption1 to take into ensure these bounds
are not overstepped.

Fixes an issue reported by Nico Golde where extra undersized data was
present in the option.
An example of this would be an array of uint16's with a trailing byte.

Fixes CVE-2016-1503
---

diff --git a/dhcp-common.c b/dhcp-common.c
index 3e064c35..d8b04a7b 100644
--- a/dhcp-common.c
+++ b/dhcp-common.c
@@ -579,48 +579,43 @@ print_string(char *dst, size_t len, int type, const uint8_t *data, size_t dl)
 	return (ssize_t)bytes;
 }
 
-#define ADDRSZ		4
 #define ADDR6SZ		16
 static ssize_t
 dhcp_optlen(const struct dhcp_opt *opt, size_t dl)
 {
 	size_t sz;
 
-	if (opt->type == 0 ||
-	    opt->type & (STRING | BINHEX | RFC3442))
-	{
+	if (opt->type & ADDRIPV6)
+		sz = ADDR6SZ;
+	else if (opt->type & (UINT32 | ADDRIPV4))
+		sz = sizeof(uint32_t);
+	else if (opt->type & UINT16)
+		sz = sizeof(uint16_t);
+	else if (opt->type & (UINT8 | BITFLAG))
+		sz = sizeof(uint8_t);
+	else if (opt->type & FLAG)
+		return 0;
+	else {
+		/* All other types are variable length */
 		if (opt->len) {
-			if ((size_t)opt->len > dl)
+			if ((size_t)opt->len > dl) {
+				errno = ENODATA;
 				return -1;
+			}
 			return (ssize_t)opt->len;
 		}
 		return (ssize_t)dl;
 	}
-
-	if ((opt->type & (ADDRIPV4 | ARRAY)) == (ADDRIPV4 | ARRAY)) {
-		if (dl < ADDRSZ)
-			return -1;
-		return (ssize_t)(dl - (dl % ADDRSZ));
-	}
-
-	if ((opt->type & (ADDRIPV6 | ARRAY)) == (ADDRIPV6 | ARRAY)) {
-		if (dl < ADDR6SZ)
-			return -1;
-		return (ssize_t)(dl - (dl % ADDR6SZ));
+	if (dl < sz) {
+		errno = ENODATA;
+		return -1;
 	}
 
-	if (opt->type & (UINT32 | ADDRIPV4))
-		sz = sizeof(uint32_t);
-	else if (opt->type & UINT16)
-		sz = sizeof(uint16_t);
-	else if (opt->type & (UINT8 | BITFLAG))
-		sz = sizeof(uint8_t);
-	else if (opt->type & ADDRIPV6)
-		sz = ADDR6SZ;
-	else
-		/* If we don't know the size, assume it's valid */
-		return (ssize_t)dl;
-	return dl < sz ? -1 : (ssize_t)sz;
+	/* Trim any extra data.
+	 * Maybe we need a settng to reject DHCP options with extra data? */
+	if (opt->type & ARRAY)
+		return (ssize_t)(dl - (dl % sz));
+	return (ssize_t)sz;
 }
 
 static ssize_t
@@ -843,8 +838,11 @@ dhcp_envoption1(char **env, const char *prefix,
 	size_t e;
 	char *v, *val;
 
-	if (opt->len && opt->len < ol)
-		ol = opt->len;
+	/* Ensure a valid length */
+	ol = (size_t)dhcp_optlen(opt, ol);
+	if ((ssize_t)ol == -1)
+		return 0;
+
 	len = print_option(NULL, 0, opt, od, ol, ifname);
 	if (len < 0)
 		return 0;