commit 20d6d4458bee5d88bda1511c225c25b2d3198d6c Author: Nick Mathewson Date: Mon Jan 5 08:42:32 2015 -0500 Fix CVE-2014-6272 in Libevent 2.0 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. Index: libevent-2.0.21-stable/buffer.c =================================================================== --- libevent-2.0.21-stable.orig/buffer.c +++ libevent-2.0.21-stable/buffer.c @@ -157,12 +157,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) @@ -1002,6 +1010,7 @@ evbuffer_drain(struct evbuffer *buf, siz buf->first = chain; if (chain) { + EVUTIL_ASSERT(remaining <= chain->off); chain->misalign += remaining; chain->off -= remaining; } @@ -1068,6 +1077,7 @@ evbuffer_copyout(struct evbuffer *buf, v if (datlen) { EVUTIL_ASSERT(chain); + EVUTIL_ASSERT(datlen <= chain->off); memcpy(data, chain->buffer + chain->misalign, datlen); } @@ -1543,6 +1553,10 @@ evbuffer_add(struct evbuffer *buf, const if (buf->freeze_end) { goto done; } + /* Prevent buf->total_len overflow */ + if (datlen > EV_SIZE_MAX - buf->total_len) { + goto done; + } chain = buf->last; @@ -1556,7 +1570,10 @@ evbuffer_add(struct evbuffer *buf, const } 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 */ @@ -1627,6 +1644,9 @@ evbuffer_prepend(struct evbuffer *buf, c if (buf->freeze_start) { goto done; } + if (datlen > EV_SIZE_MAX - buf->total_len) { + goto done; + } chain = buf->first; @@ -1639,6 +1659,10 @@ evbuffer_prepend(struct evbuffer *buf, c /* 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) @@ -1676,6 +1700,7 @@ evbuffer_prepend(struct evbuffer *buf, c 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); @@ -1774,7 +1799,9 @@ evbuffer_expand_singlechain(struct evbuf /* 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) { @@ -1902,6 +1929,8 @@ _evbuffer_expand_fast(struct evbuffer *b 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; } @@ -1912,6 +1941,7 @@ _evbuffer_expand_fast(struct evbuffer *b 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) { @@ -2041,6 +2071,7 @@ get_n_bytes_readable_on_socket(evutil_so 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; @@ -2153,8 +2184,14 @@ evbuffer_read(struct evbuffer *buf, evut #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 { @@ -2425,12 +2462,17 @@ evbuffer_ptr_set(struct evbuffer *buf, s 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; @@ -2463,7 +2505,9 @@ evbuffer_ptr_memcmp(const struct evbuffe 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; @@ -2651,6 +2695,9 @@ evbuffer_add_vprintf(struct evbuffer *bu 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; @@ -2744,6 +2791,11 @@ evbuffer_add_file(struct evbuffer *outbu #endif int ok = 1; + if (offset < 0 || length < 0 || + ((ev_uint64_t)length > EVBUFFER_CHAIN_MAX) || + (ev_uint64_t)offset > (ev_uint64_t)(EVBUFFER_CHAIN_MAX - length)) + return (-1); + #if defined(USE_SENDFILE) if (use_sendfile) { EVBUFFER_LOCK(outbuf); @@ -2849,7 +2901,8 @@ evbuffer_add_file(struct evbuffer *outbu * can abort without side effects if the read fails. */ while (length) { - read = evbuffer_readfile(tmp, fd, (ev_ssize_t)length); + ev_ssize_t to_read = length > EV_SSIZE_MAX ? EV_SSIZE_MAX : (ev_ssize_t)length; + read = evbuffer_readfile(tmp, fd, to_read); if (read == -1) { evbuffer_free(tmp); return (-1); Index: libevent-2.0.21-stable/configure.in =================================================================== --- libevent-2.0.21-stable.orig/configure.in +++ libevent-2.0.21-stable/configure.in @@ -553,6 +553,7 @@ AC_CHECK_SIZEOF(int) AC_CHECK_SIZEOF(short) AC_CHECK_SIZEOF(size_t) AC_CHECK_SIZEOF(void *) +AC_CHECK_SIZEOF(off_t) AC_CHECK_TYPES([struct in6_addr, struct sockaddr_in6, sa_family_t, struct addrinfo, struct sockaddr_storage], , , [#define _GNU_SOURCE Index: libevent-2.0.21-stable/evbuffer-internal.h =================================================================== --- libevent-2.0.21-stable.orig/evbuffer-internal.h +++ libevent-2.0.21-stable/evbuffer-internal.h @@ -153,6 +153,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 */ @@ -163,7 +175,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