From fec195b56b97e093d7aeffbb0b504fbc9971bdc2 Mon Sep 17 00:00:00 2001
From: Roy Marples <roy@marples.name>
Date: Tue, 5 Aug 2014 17:03:04 +0000
Subject: [PATCH] Check snprintf doesn't overflow in dhcp_vendor and that a -1
 return is correctly handled.

Thanks to Tobias Stoeckmann.
---
 dhcp-common.c | 10 +++++++---
 dhcp-common.h |  2 +-
 dhcp.c        |  1 -
 dhcp6.c       | 13 ++++++++-----
 if-options.c  |  7 ++++---
 5 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/dhcp-common.c b/dhcp-common.c
index d8ffc5dd..f2e1b5e0 100644
--- a/dhcp-common.c
+++ b/dhcp-common.c
@@ -63,7 +63,7 @@ vivso_find(uint32_t iana_en, const void *arg)
 	return NULL;
 }
 
-size_t
+ssize_t
 dhcp_vendor(char *str, size_t len)
 {
 	struct utsname utn;
@@ -71,17 +71,21 @@ dhcp_vendor(char *str, size_t len)
 	int l;
 
 	if (uname(&utn) != 0)
-		return (size_t)snprintf(str, len, "%s-%s",
+		return (ssize_t)snprintf(str, len, "%s-%s",
 		    PACKAGE, VERSION);
 	p = str;
 	l = snprintf(p, len,
 	    "%s-%s:%s-%s:%s", PACKAGE, VERSION,
 	    utn.sysname, utn.release, utn.machine);
+	if (l == -1 || (size_t)(l + 1) > len)
+		return -1;
 	p += l;
 	len -= (size_t)l;
 	l = if_machinearch(p, len);
+	if (l == -1 || (size_t)(l + 1) > len)
+		return -1;
 	p += l;
-	return (size_t)(p - str);
+	return p - str;
 }
 
 int
diff --git a/dhcp-common.h b/dhcp-common.h
index e0994bf8..5519edac 100644
--- a/dhcp-common.h
+++ b/dhcp-common.h
@@ -82,7 +82,7 @@ struct dhcp_opt {
 
 struct dhcp_opt *vivso_find(uint32_t, const void *);
 
-size_t dhcp_vendor(char *, size_t);
+ssize_t dhcp_vendor(char *, size_t);
 
 #define add_option_mask(var, val) (var[val >> 3] |= 1 << (val & 7))
 #define del_option_mask(var, val) (var[val >> 3] &= ~(1 << (val & 7)))
diff --git a/dhcp.c b/dhcp.c
index 13a57592..4b63e178 100644
--- a/dhcp.c
+++ b/dhcp.c
@@ -828,7 +828,6 @@ make_message(struct dhcp_message **message,
 			p += ifo->vendorclassid[0] + 1;
 		}
 
-
 		if (type != DHCP_INFORM) {
 			if (ifo->leasetime != 0) {
 				*p++ = DHO_LEASETIME;
diff --git a/dhcp6.c b/dhcp6.c
index b1f814aa..86337cd0 100644
--- a/dhcp6.c
+++ b/dhcp6.c
@@ -156,11 +156,11 @@ static size_t
 dhcp6_makevendor(struct dhcp6_option *o, const struct interface *ifp)
 {
 	const struct if_options *ifo;
-	size_t len;
+	size_t len, i;
 	uint8_t *p;
 	uint16_t u16;
 	uint32_t u32;
-	size_t vlen, i;
+	ssize_t vlen;
 	const struct vivco *vivco;
 	char vendor[VENDORCLASSID_MAX_LEN];
 
@@ -174,7 +174,10 @@ dhcp6_makevendor(struct dhcp6_option *o, const struct interface *ifp)
 		vlen = 0; /* silence bogus gcc warning */
 	} else {
 		vlen = dhcp_vendor(vendor, sizeof(vendor));
-		len += sizeof(uint16_t) + vlen;
+		if (vlen == -1)
+			vlen = 0;
+		else
+			len += sizeof(uint16_t) + (size_t)vlen;
 	}
 
 	if (len > UINT16_MAX) {
@@ -200,11 +203,11 @@ dhcp6_makevendor(struct dhcp6_option *o, const struct interface *ifp)
 				memcpy(p, vivco->data, vivco->len);
 				p += vivco->len;
 			}
-		} else {
+		} else if (vlen) {
 			u16 = htons(vlen);
 			memcpy(p, &u16, sizeof(u16));
 			p += sizeof(u16);
-			memcpy(p, vendor, vlen);
+			memcpy(p, vendor, (size_t)vlen);
 		}
 	}
 
diff --git a/if-options.c b/if-options.c
index 4e921ba8..d8bb8bee 100644
--- a/if-options.c
+++ b/if-options.c
@@ -1981,6 +1981,7 @@ read_config(struct dhcpcd_ctx *ctx,
 	FILE *fp;
 	char *line, *buf, *option, *p;
 	size_t buflen;
+	ssize_t vlen;
 	int skip = 0, have_profile = 0;
 #ifndef EMBEDDED_CONFIG
 	const char * const *e;
@@ -2016,9 +2017,9 @@ read_config(struct dhcpcd_ctx *ctx,
 	ifo->auth.options |= DHCPCD_AUTH_REQUIRE;
 	TAILQ_INIT(&ifo->auth.tokens);
 
-	ifo->vendorclassid[0] =
-	    (uint8_t)dhcp_vendor((char *)ifo->vendorclassid + 1,
-	    sizeof(ifo->vendorclassid) - 1);
+	vlen = dhcp_vendor((char *)ifo->vendorclassid + 1,
+	            sizeof(ifo->vendorclassid) - 1);
+	ifo->vendorclassid[0] = vlen == -1 ? 0 : (uint8_t)vlen;
 
 	buf = NULL;
 	buflen = 0;
-- 
2.47.3