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;
>