Fix CVE-2014-6272 in Libevent 2.1

For this fix, we need to make sure that passing too-large inputs to
the evbuffer functions can't make us do bad things with the heap.

Also, lower the maximum chunk size to the lower of off_t, size_t maximum.

This is necessary since otherwise we could get into an infinite loop
if we make a chunk that 'misalign' cannot index into.

2 files changed, 80 insertions, 9 deletions

diff --git a/buffer.c b/buffer.c
index 231f1914..a1a2b988 100644
--- a/buffer.c
+++ b/buffer.c
@@ -158,12 +158,20 @@ evbuffer_chain_new(size_t size)
 	struct evbuffer_chain *chain;
 	size_t to_alloc;
 
+	if (size > EVBUFFER_CHAIN_MAX - EVBUFFER_CHAIN_SIZE)
+		return (NULL);
+
 	size += EVBUFFER_CHAIN_SIZE;
 
 	/* get the next largest memory that can hold the buffer */
-	to_alloc = MIN_BUFFER_SIZE;
-	while (to_alloc < size)
-		to_alloc <<= 1;
+	if (size < EVBUFFER_CHAIN_MAX / 2) {
+		to_alloc = MIN_BUFFER_SIZE;
+		while (to_alloc < size) {
+			to_alloc <<= 1;
+		}
+	} else {
+		to_alloc = size;
+	}
 
 	/* we get everything in one chunk */
 	if ((chain = mm_malloc(to_alloc)) == NULL)
@@ -1133,6 +1141,7 @@ evbuffer_drain(struct evbuffer *buf, size_t len)
 		}
 
 		buf->first = chain;
+		EVUTIL_ASSERT(chain && remaining <= chain->off);
 		chain->misalign += remaining;
 		chain->off -= remaining;
 	}
@@ -1181,6 +1190,10 @@ evbuffer_copyout_from(struct evbuffer *buf, const struct evbuffer_ptr *pos,
 	EVBUFFER_LOCK(buf);
 
 	if (pos) {
+		if (datlen > (size_t)(EV_SSIZE_MAX - pos->pos)) {
+			result = -1;
+			goto done;
+		}
 		chain = pos->internal_.chain;
 		pos_in_chain = pos->internal_.pos_in_chain;
 		if (datlen + pos->pos > buf->total_len)
@@ -1218,6 +1231,8 @@ evbuffer_copyout_from(struct evbuffer *buf, const struct evbuffer_ptr *pos,
 
 	if (datlen) {
 		EVUTIL_ASSERT(chain);
+		EVUTIL_ASSERT(datlen+pos_in_chain <= chain->off);
+
 		memcpy(data, chain->buffer + chain->misalign + pos_in_chain,
 		    datlen);
 	}
@@ -1712,6 +1727,10 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen)
 	if (buf->freeze_end) {
 		goto done;
 	}
+	/* Prevent buf->total_len overflow */
+	if (datlen > EV_SIZE_MAX - buf->total_len) {
+		goto done;
+	}
 
 	chain = buf->last;
 
@@ -1725,7 +1744,10 @@ evbuffer_add(struct evbuffer *buf, const void *data_in, size_t datlen)
 	}
 
 	if ((chain->flags & EVBUFFER_IMMUTABLE) == 0) {
-		remain = (size_t)(chain->buffer_len - chain->misalign - chain->off);
+		/* Always true for mutable buffers */
+		EVUTIL_ASSERT(chain->misalign >= 0 &&
+		    (ev_uint64_t)chain->misalign <= EVBUFFER_CHAIN_MAX);
+		remain = chain->buffer_len - (size_t)chain->misalign - chain->off;
 		if (remain >= datlen) {
 			/* there's enough space to hold all the data in the
 			 * current last chain */
@@ -1796,6 +1818,9 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen)
 	if (buf->freeze_start) {
 		goto done;
 	}
+	if (datlen > EV_SIZE_MAX - buf->total_len) {
+		goto done;
+	}
 
 	chain = buf->first;
 
@@ -1808,6 +1833,10 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen)
 
 	/* we cannot touch immutable buffers */
 	if ((chain->flags & EVBUFFER_IMMUTABLE) == 0) {
+		/* Always true for mutable buffers */
+		EVUTIL_ASSERT(chain->misalign >= 0 &&
+		    (ev_uint64_t)chain->misalign <= EVBUFFER_CHAIN_MAX);
+
 		/* If this chain is empty, we can treat it as
 		 * 'empty at the beginning' rather than 'empty at the end' */
 		if (chain->off == 0)
@@ -1845,6 +1874,7 @@ evbuffer_prepend(struct evbuffer *buf, const void *data, size_t datlen)
 	tmp->next = chain;
 
 	tmp->off = datlen;
+	EVUTIL_ASSERT(datlen <= tmp->buffer_len);
 	tmp->misalign = tmp->buffer_len - datlen;
 
 	memcpy(tmp->buffer + tmp->misalign, data, datlen);
@@ -1943,7 +1973,9 @@ evbuffer_expand_singlechain(struct evbuffer *buf, size_t datlen)
 
 	/* Would expanding this chunk be affordable and worthwhile? */
 	if (CHAIN_SPACE_LEN(chain) < chain->buffer_len / 8 ||
-	    chain->off > MAX_TO_COPY_IN_EXPAND) {
+	    chain->off > MAX_TO_COPY_IN_EXPAND ||
+	    (datlen < EVBUFFER_CHAIN_MAX &&
+		EVBUFFER_CHAIN_MAX - datlen >= chain->off)) {
 		/* It's not worth resizing this chain. Can the next one be
 		 * used? */
 		if (chain->next && CHAIN_SPACE_LEN(chain->next) >= datlen) {
@@ -2071,6 +2103,8 @@ evbuffer_expand_fast_(struct evbuffer *buf, size_t datlen, int n)
 			rmv_all = 1;
 			avail = 0;
 		} else {
+			/* can't overflow, since only mutable chains have
+			 * huge misaligns. */
 			avail = (size_t) CHAIN_SPACE_LEN(chain);
 			chain = chain->next;
 		}
@@ -2081,6 +2115,7 @@ evbuffer_expand_fast_(struct evbuffer *buf, size_t datlen, int n)
 			EVUTIL_ASSERT(chain->off == 0);
 			evbuffer_chain_free(chain);
 		}
+		EVUTIL_ASSERT(datlen >= avail);
 		tmp = evbuffer_chain_new(datlen - avail);
 		if (tmp == NULL) {
 			if (rmv_all) {
@@ -2210,6 +2245,7 @@ get_n_bytes_readable_on_socket(evutil_socket_t fd)
 	unsigned long lng = EVBUFFER_MAX_READ;
 	if (ioctlsocket(fd, FIONREAD, &lng) < 0)
 		return -1;
+	/* Can overflow, but mostly harmlessly. XXXX */
 	return (int)lng;
 #elif defined(FIONREAD)
 	int n = EVBUFFER_MAX_READ;
@@ -2322,8 +2358,14 @@ evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch)
 #ifdef USE_IOVEC_IMPL
 	remaining = n;
 	for (i=0; i < nvecs; ++i) {
-		ev_ssize_t space = (ev_ssize_t) CHAIN_SPACE_LEN(*chainp);
-		if (space < remaining) {
+		/* can't overflow, since only mutable chains have
+		 * huge misaligns. */
+		size_t space = (size_t) CHAIN_SPACE_LEN(*chainp);
+		/* XXXX This is a kludge that can waste space in perverse
+		 * situations. */
+		if (space > EVBUFFER_CHAIN_MAX)
+			space = EVBUFFER_CHAIN_MAX;
+		if ((ev_ssize_t)space < remaining) {
 			(*chainp)->off += space;
 			remaining -= (int)space;
 		} else {
@@ -2540,6 +2582,8 @@ static int
 evbuffer_ptr_subtract(struct evbuffer *buf, struct evbuffer_ptr *pos,
     size_t howfar)
 {
+	if (pos->pos < 0)
+		return -1;
 	if (howfar > (size_t)pos->pos)
 		return -1;
 	if (pos->internal_.chain && howfar <= pos->internal_.pos_in_chain) {
@@ -2573,12 +2617,17 @@ evbuffer_ptr_set(struct evbuffer *buf, struct evbuffer_ptr *pos,
 	case EVBUFFER_PTR_ADD:
 		/* this avoids iterating over all previous chains if
 		   we just want to advance the position */
+		if (pos->pos < 0 || EV_SIZE_MAX - position < (size_t)pos->pos) {
+			EVBUFFER_UNLOCK(buf);
+			return -1;
+		}
 		chain = pos->internal_.chain;
 		pos->pos += position;
 		position = pos->internal_.pos_in_chain;
 		break;
 	}
 
+	EVUTIL_ASSERT(EV_SIZE_MAX - left >= position);
 	while (chain && position + left >= chain->off) {
 		left -= chain->off - position;
 		chain = chain->next;
@@ -2615,7 +2664,9 @@ evbuffer_ptr_memcmp(const struct evbuffer *buf, const struct evbuffer_ptr *pos,
 
 	ASSERT_EVBUFFER_LOCKED(buf);
 
-	if (pos->pos + len > buf->total_len)
+	if (pos->pos < 0 ||
+	    EV_SIZE_MAX - len < (size_t)pos->pos ||
+	    pos->pos + len > buf->total_len)
 		return -1;
 
 	chain = pos->internal_.chain;
@@ -2809,6 +2860,9 @@ evbuffer_add_vprintf(struct evbuffer *buf, const char *fmt, va_list ap)
 
 		if (sz < 0)
 			goto done;
+		if (INT_MAX >= EVBUFFER_CHAIN_MAX &&
+		    (size_t)sz >= EVBUFFER_CHAIN_MAX)
+			goto done;
 		if ((size_t)sz < space) {
 			chain->off += sz;
 			buf->total_len += sz;
@@ -2918,6 +2972,11 @@ evbuffer_file_segment_new(
 	}
 	seg->length = length;
 
+	if (offset < 0 || length < 0 ||
+	    ((ev_uint64_t)length > EVBUFFER_CHAIN_MAX) ||
+	    (ev_uint64_t)offset > (ev_uint64_t)(EVBUFFER_CHAIN_MAX - length))
+		goto err;
+
 #if defined(USE_SENDFILE)
 	if (!(flags & EVBUF_FS_DISABLE_SENDFILE)) {
 		seg->can_sendfile = 1;
diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index fb67ec09..cf4bddc8 100644
--- a/evbuffer-internal.h
+++ b/evbuffer-internal.h
@@ -155,6 +155,18 @@ struct evbuffer {
 	struct bufferevent *parent;
 };
 
+#if EVENT__SIZEOF_OFF_T < EVENT__SIZEOF_SIZE_T
+typedef ev_ssize_t ev_misalign_t;
+#define EVBUFFER_CHAIN_MAX ((size_t)EV_SSIZE_MAX)
+#else
+typedef ev_off_t ev_misalign_t;
+#if EVENT__SIZEOF_OFF_T > EVENT__SIZEOF_SIZE_T
+#define EVBUFFER_CHAIN_MAX EV_SIZE_MAX
+#else
+#define EVBUFFER_CHAIN_MAX ((size_t)EV_SSIZE_MAX)
+#endif
+#endif
+
 /** A single item in an evbuffer. */
 struct evbuffer_chain {
 	/** points to next buffer in the chain */
@@ -165,7 +177,7 @@ struct evbuffer_chain {
 
 	/** unused space at the beginning of buffer or an offset into a
 	 * file for sendfile buffers. */
-	ev_off_t misalign;
+	ev_misalign_t misalign;
 
 	/** Offset into buffer + misalign at which to start writing.
 	 * In other words, the total number of bytes actually stored