ok nicm



On Thu, May 02, 2019 at 06:59:33PM +0200, Tobias Stöckmann wrote:
> It is possible to trigger an endless loop or out of boundary write
> on 64 bit systems with evbuffer_readline calls for buffers which
> exceed 4 GB (i.e. overflow uint).
> 
>       for (i = 0; i < len; i++)
> 
> Variable i is unsigned int and len size_t. This leads to an endless
> loop if len is larger than UINT_MAX.
> 
> If the loop ends exactly at UINT_MAX, then a malloc with additional
> space for an ending '\0' leads to an overflow, effectively allocating
> 0 bytes and therefore an out of boundary write occurs.
> 
> This is a proof of concept for an endless loop:
> 
> #include <err.h>
> #include <event.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> 
> int
> main(void)
> {
>       char *buf;
>       const size_t len = 0x100000000UL;
>       struct evbuffer *buffer;
>       char *line;
> 
>       if ((buf = malloc(len)) == NULL)
>               err(1, "malloc");
>       memset(buf, 'A', len);
> 
>       if ((buffer = evbuffer_new()) == NULL)
>               errx(1, "evbuffer_new");
>       if (evbuffer_expand(buffer, len + 1))
>               errx(1, "evbuffer_expand");
>       evbuffer_add(buffer, buf, len);
>       evbuffer_add(buffer, "", 1);
> 
>       printf("%p\n", evbuffer_readline(buffer));
>       return 0;
> }
> 
> Generally this is a rather theoretical case. Normal users are not
> allowed to allocate so much memory. But better be safe than sorry,
> especially if login.conf values were adjusted (or the process runs
> as root).
> 
> This patch completely removes "unsigned int" from buffer.c.
> 
> 
> Index: buffer.c
> ===================================================================
> RCS file: /cvs/src/lib/libevent/buffer.c,v
> retrieving revision 1.31
> diff -u -p -u -p -r1.31 buffer.c
> --- buffer.c  18 Mar 2017 01:48:43 -0000      1.31
> +++ buffer.c  1 May 2019 11:00:29 -0000
> @@ -188,7 +188,7 @@ evbuffer_readline(struct evbuffer *buffe
>       u_char *data = EVBUFFER_DATA(buffer);
>       size_t len = EVBUFFER_LENGTH(buffer);
>       char *line;
> -     unsigned int i;
> +     size_t i;
> 
>       for (i = 0; i < len; i++) {
>               if (data[i] == '\r' || data[i] == '\n')
> @@ -232,7 +232,7 @@ evbuffer_readln(struct evbuffer *buffer,
>       u_char *start_of_eol, *end_of_eol;
>       size_t len = EVBUFFER_LENGTH(buffer);
>       char *line;
> -     unsigned int i, n_to_copy, n_to_drain;
> +     size_t i, n_to_copy, n_to_drain;
> 
>       if (n_read_out)
>               *n_read_out = 0;
> 

Reply via email to