From 841ecbd96105c84ac2e7c9594aeadbcc6fb38bc4 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Mon, 5 Jan 2015 09:32:53 -0500 Subject: 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. --- buffer.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++------ evbuffer-internal.h | 14 +++++++++- 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 -- cgit v1.2.1