libevent: endless loop on excessively large buffers

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

libevent: endless loop on excessively large buffers

Tobias Stoeckmann-5
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 | Threaded
Open this post in threaded view
|

Re: libevent: endless loop on excessively large buffers

Ted Unangst-6
Tobias Stöckmann wrote:
> 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.

yes please.

Reply | Threaded
Open this post in threaded view
|

Re: libevent: endless loop on excessively large buffers

Nicholas Marriott-2
In reply to this post by Tobias Stoeckmann-5

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