From: Ben Hutchings <ben@decadent.org.uk>
Date: Fri, 22 Jan 2016 18:02:04 +0000 (+0000)
Subject: Fix potential buffer overflow in entry_to_env
X-Git-Tag: debian/1.1+git20160131-1~8^2~7
X-Git-Url: https://git.decadent.org.uk/gitweb/?p=odhcp6c.git;a=commitdiff_plain;h=a6bbd1d7f5c25b092f143b579860a44e5b0f929e

Fix potential buffer overflow in entry_to_env

It appears that an entry of type ENTRY_PREFIX with iaid != 1 and an
exclusion can expand to a string of length up to 154 bytes, whereas we
allocate only 144 bytes per entry.

Also, in case of truncation, snprintf() returns the length of the
un-truncated output so we must not use this to increment buf_len.

Finally some of the lengths given to snprintf() are unnecessarily
generous.  Reduce them so we don't have to increase the allocated
length per entry further.

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---

diff --git a/src/script.c b/src/script.c
index 49f39c4..83fbea5 100644
--- a/src/script.c
+++ b/src/script.c
@@ -157,7 +157,10 @@ static void entry_to_env(const char *name, const void *data, size_t len, enum en
 {
 	size_t buf_len = strlen(name);
 	const struct odhcp6c_entry *e = data;
-	char *buf = realloc(NULL, buf_len + 2 + (len / sizeof(*e)) * 144);
+	// Worst case: ENTRY_PREFIX with iaid != 1 and exclusion
+	const size_t max_entry_len = (INET6_ADDRSTRLEN-1 + 5 + 22 + 15 + 10 +
+				      INET6_ADDRSTRLEN-1 + 11 + 1);
+	char *buf = realloc(NULL, buf_len + 2 + (len / sizeof(*e)) * max_entry_len);
 	memcpy(buf, name, buf_len);
 	buf[buf_len++] = '=';
 
@@ -165,28 +168,34 @@ static void entry_to_env(const char *name, const void *data, size_t len, enum en
 		inet_ntop(AF_INET6, &e[i].target, &buf[buf_len], INET6_ADDRSTRLEN);
 		buf_len += strlen(&buf[buf_len]);
 		if (type != ENTRY_HOST) {
-			buf_len += snprintf(&buf[buf_len], 6, "/%"PRIu16, e[i].length);
+			snprintf(&buf[buf_len], 6, "/%"PRIu16, e[i].length);
+			buf += strlen(&buf[buf_len]);
 			if (type == ENTRY_ROUTE) {
 				buf[buf_len++] = ',';
 				if (!IN6_IS_ADDR_UNSPECIFIED(&e[i].router)) {
 					inet_ntop(AF_INET6, &e[i].router, &buf[buf_len], INET6_ADDRSTRLEN);
 					buf_len += strlen(&buf[buf_len]);
 				}
-				buf_len += snprintf(&buf[buf_len], 24, ",%u", e[i].valid);
-				buf_len += snprintf(&buf[buf_len], 12, ",%u", e[i].priority);
+				snprintf(&buf[buf_len], 23, ",%u,%u", e[i].valid, e[i].priority);
+				buf += strlen(&buf[buf_len]);
 			} else {
-				buf_len += snprintf(&buf[buf_len], 24, ",%u,%u", e[i].preferred, e[i].valid);
+				snprintf(&buf[buf_len], 23, ",%u,%u", e[i].preferred, e[i].valid);
+				buf += strlen(&buf[buf_len]);
 			}
 
-			if (type == ENTRY_PREFIX && ntohl(e[i].iaid) != 1)
-				buf_len += snprintf(&buf[buf_len], 16, ",class=%08x", ntohl(e[i].iaid));
+			if (type == ENTRY_PREFIX && ntohl(e[i].iaid) != 1) {
+				snprintf(&buf[buf_len], 16, ",class=%08x", ntohl(e[i].iaid));
+				buf += strlen(&buf[buf_len]);
+			}
 
 			if (type == ENTRY_PREFIX && e[i].priority) {
 				// priority and router are abused for prefix exclusion
-				buf_len += snprintf(&buf[buf_len], 12, ",excluded=");
+				snprintf(&buf[buf_len], 11, ",excluded=");
+				buf_len += strlen(&buf[buf_len]);
 				inet_ntop(AF_INET6, &e[i].router, &buf[buf_len], INET6_ADDRSTRLEN);
 				buf_len += strlen(&buf[buf_len]);
-				buf_len += snprintf(&buf[buf_len], 24, "/%u", e[i].priority);
+				snprintf(&buf[buf_len], 12, "/%u", e[i].priority);
+				buf_len += strlen(&buf[buf_len]);
 			}
 		}
 		buf[buf_len++] = ' ';