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;
