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.

diff --git a/buffer.c b/buffer.c
index 388615cd4fd412bd1d1536f8a2f7392a0bff20fc..8510955b7b9d6f7997456220f7348b4a064ba806 100644 (file)
--- a/buffer.c
+++ b/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, size_t len)
 
                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, void *data_out, size_t datlen)
 
        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 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;
 
@@ -1556,7 +1570,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 */
@@ -1627,6 +1644,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;
 
@@ -1639,6 +1659,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)
@@ -1676,6 +1700,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);
@@ -1774,7 +1799,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) {
@@ -1902,6 +1929,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;
                }
@@ -1912,6 +1941,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) {
@@ -2041,6 +2071,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;
@@ -2153,8 +2184,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 {
@@ -2427,12 +2464,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;
@@ -2465,7 +2507,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;
@@ -2656,6 +2700,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;
@@ -2749,6 +2796,11 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
 #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);
@@ -2854,7 +2906,8 @@ evbuffer_add_file(struct evbuffer *outbuf, int fd,
                 * 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);

diff --git a/configure.ac b/configure.ac
index a00a767749e6498fe1b747c03a84317c29411b0b..55652aa61575b3ac4f7a05c08d2397e6cb0a99da 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -558,6 +558,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

diff --git a/evbuffer-internal.h b/evbuffer-internal.h
index e68a59df6e9d813f3d8d83d383291f364f0dd791..f9dcc06125e72b63047fb90b970fd21d2eec0a44 100644 (file)
--- a/evbuffer-internal.h
+++ b/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