Skip to content

Commit

Permalink
Forbid 0x/+/-/whitespace prefixes on HTTP chunk sizes (libevent#1542)
Browse files Browse the repository at this point in the history
Currently, libevent's HTTP parser accepts and ignores 0x, +, and whitespace prefixes on chunk sizes. It also ignores - prefixes on chunk sizes of 0. This patch fixes that.

There is a potential danger in the current behavior, which is that there exist HTTP implementations that interpret chunk sizes as their longest valid prefix. For those implementations, 0xa (for example) is equivalent to 0, and this may present a request smuggling risk when those implementations are used in conjunction with libevent. However, as far I'm aware, there is no HTTP proxy that both interprets 0xa as 0 and forwards it verbatim, so I think this is a low-risk bug that is acceptable to report in public.
  • Loading branch information
kenballus authored Feb 18, 2024
1 parent da3c7b5 commit 92ea847
Showing 1 changed file with 13 additions and 1 deletion.
14 changes: 13 additions & 1 deletion http.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
#include <winsock2.h>
#endif

#include <ctype.h>
#include <errno.h>
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -1028,13 +1029,24 @@ evhttp_handle_chunked_read(struct evhttp_request *req, struct evbuffer *buf)
char *p = evbuffer_readln(buf, NULL, EVBUFFER_EOL_CRLF);
char *endp;
int error;
size_t len_p;
if (p == NULL)
break;
len_p = strlen(p);
/* the last chunk is on a new line? */
if (strlen(p) == 0) {
if (len_p == 0) {
mm_free(p);
continue;
}
/* strtoll(,,16) lets through whitespace, 0x, +, and - prefixes, but HTTP doesn't. */
error = isspace(p[0]) ||
p[0] == '-' ||
p[0] == '+' ||
(len_p >= 2 && p[1] == 'x');
if (error) {
mm_free(p);
return (DATA_CORRUPTED);
}
ntoread = evutil_strtoll(p, &endp, 16);
error = (*p == '\0' ||
(*endp != '\0' && *endp != ' ') ||
Expand Down

0 comments on commit 92ea847

Please sign in to comment.