changeset 3409:6838b85e6c6a draft

Check return value of snprintf and ipv6_printaddr. This is not really needed (unless you have a buggy libc) as the correct bounds are checked elsewhere, but it's hard to see that, hence this change. Fixes CVE-2014-7913.
author Roy Marples <roy@marples.name>
date Fri, 22 Jan 2016 10:05:47 +0000
parents df62940aacb7
children 8f4e7b9d5472
files dhcp-common.c ipv6.c ipv6.h
diffstat 3 files changed, 46 insertions(+), 59 deletions(-) [+]
line wrap: on
line diff
--- a/dhcp-common.c	Wed Jan 20 19:08:12 2016 +0000
+++ b/dhcp-common.c	Fri Jan 22 10:05:47 2016 +0000
@@ -618,6 +618,39 @@
 	return (ssize_t)sz;
 }
 
+/* It's possible for DHCPv4 to contain an IPv6 address */
+static ssize_t
+ipv6_printaddr(char *s, size_t sl, const uint8_t *d, const char *ifname)
+{
+	char buf[INET6_ADDRSTRLEN];
+	const char *p;
+	size_t l;
+
+	p = inet_ntop(AF_INET6, d, buf, sizeof(buf));
+	if (p == NULL)
+		return -1;
+
+	l = strlen(p);
+	if (d[0] == 0xfe && (d[1] & 0xc0) == 0x80)
+		l += 1 + strlen(ifname);
+
+	if (s == NULL)
+		return (ssize_t)l;
+
+	if (sl < l) {
+		errno = ENOMEM;
+		return -1;
+	}
+
+	s += strlcpy(s, p, sl);
+	if (d[0] == 0xfe && (d[1] & 0xc0) == 0x80) {
+		*s++ = '%';
+		s += strlcpy(s, ifname, sl);
+	}
+	*s = '\0';
+	return (ssize_t)l;
+}
+
 static ssize_t
 print_option(char *s, size_t len, const struct dhcp_opt *opt,
     const uint8_t *data, size_t dl, const char *ifname)
@@ -632,10 +665,6 @@
 	size_t l;
 	char *tmp;
 
-#ifndef INET6
-	UNUSED(ifname);
-#endif
-
 	if (opt->type & RFC1035) {
 		sl = decode_rfc1035(NULL, 0, data, dl);
 		if (sl == 0 || sl == -1)
@@ -716,23 +745,20 @@
 		} else if (opt->type & ADDRIPV4) {
 			l = 16;
 			dl /= 4;
-		}
-#ifdef INET6
-		else if (opt->type & ADDRIPV6) {
+		} else if (opt->type & ADDRIPV6) {
 			e = data + dl;
 			l = 0;
 			while (data < e) {
 				if (l)
 					l++; /* space */
 				sl = ipv6_printaddr(NULL, 0, data, ifname);
-				if (sl != -1)
-					l += (size_t)sl;
+				if (sl == -1)
+					return l == 0 ? -1 : (ssize_t)l;
+				l += (size_t)sl;
 				data += 16;
 			}
 			return (ssize_t)l;
-		}
-#endif
-		else {
+		} else {
 			errno = EINVAL;
 			return -1;
 		}
@@ -774,21 +800,15 @@
 			memcpy(&addr.s_addr, data, sizeof(addr.s_addr));
 			sl = snprintf(s, len, "%s", inet_ntoa(addr));
 			data += sizeof(addr.s_addr);
+		} else if (opt->type & ADDRIPV6) {
+			sl = ipv6_printaddr(s, len, data, ifname);
+			data += 16;
+		} else {
+			errno = EINVAL;
+			return -1;
 		}
-#ifdef INET6
-		else if (opt->type & ADDRIPV6) {
-			ssize_t r;
-
-			r = ipv6_printaddr(s, len, data, ifname);
-			if (r != -1)
-				sl = r;
-			else
-				sl = 0;
-			data += 16;
-		}
-#endif
-		else
-			sl = 0;
+		if (sl == -1)
+			return bytes == 0 ? -1 : bytes;
 		len -= (size_t)sl;
 		bytes += sl;
 		s += sl;
--- a/ipv6.c	Wed Jan 20 19:08:12 2016 +0000
+++ b/ipv6.c	Fri Jan 22 10:05:47 2016 +0000
@@ -181,38 +181,6 @@
 	return ctx;
 }
 
-ssize_t
-ipv6_printaddr(char *s, size_t sl, const uint8_t *d, const char *ifname)
-{
-	char buf[INET6_ADDRSTRLEN];
-	const char *p;
-	size_t l;
-
-	p = inet_ntop(AF_INET6, d, buf, sizeof(buf));
-	if (p == NULL)
-		return -1;
-
-	l = strlen(p);
-	if (d[0] == 0xfe && (d[1] & 0xc0) == 0x80)
-		l += 1 + strlen(ifname);
-
-	if (s == NULL)
-		return (ssize_t)l;
-
-	if (sl < l) {
-		errno = ENOMEM;
-		return -1;
-	}
-
-	s += strlcpy(s, p, sl);
-	if (d[0] == 0xfe && (d[1] & 0xc0) == 0x80) {
-		*s++ = '%';
-		s += strlcpy(s, ifname, sl);
-	}
-	*s = '\0';
-	return (ssize_t)l;
-}
-
 static ssize_t
 ipv6_readsecret(struct dhcpcd_ctx *ctx)
 {
--- a/ipv6.h	Wed Jan 20 19:08:12 2016 +0000
+++ b/ipv6.h	Fri Jan 22 10:05:47 2016 +0000
@@ -236,7 +236,6 @@
 };
 
 struct ipv6_ctx *ipv6_init(struct dhcpcd_ctx *);
-ssize_t ipv6_printaddr(char *, size_t, const uint8_t *, const char *);
 int ipv6_makestableprivate(struct in6_addr *addr,
     const struct in6_addr *prefix, int prefix_len,
     const struct interface *ifp, int *dad_counter);