Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Commit 9ce7316

Browse files
committed
src: fix out-of-bounds read through strtoul
`strtoul` will attempt to lookup the next digit up until it will stumble upon an invalid one. However, for an unterminated string as an input value, this results in out-of-bounds read. Remove `strtoul` call, and replace it with simple loop. Fix: #408 PR-URL: #409 Reviewed-By: Ben Noordhuis <[email protected]>
1 parent b11de0f commit 9ce7316

File tree

2 files changed

+46
-7
lines changed

2 files changed

+46
-7
lines changed

http_parser.c

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
#include <assert.h>
2323
#include <stddef.h>
2424
#include <ctype.h>
25-
#include <stdlib.h>
2625
#include <string.h>
2726
#include <limits.h>
2827

@@ -2367,12 +2366,27 @@ http_parser_parse_url(const char *buf, size_t buflen, int is_connect,
23672366
}
23682367

23692368
if (u->field_set & (1 << UF_PORT)) {
2370-
/* Don't bother with endp; we've already validated the string */
2371-
unsigned long v = strtoul(buf + u->field_data[UF_PORT].off, NULL, 10);
2372-
2373-
/* Ports have a max value of 2^16 */
2374-
if (v > 0xffff) {
2375-
return 1;
2369+
uint16_t off;
2370+
uint16_t len;
2371+
const char* p;
2372+
const char* end;
2373+
unsigned long v;
2374+
2375+
off = u->field_data[UF_PORT].off;
2376+
len = u->field_data[UF_PORT].len;
2377+
end = buf + off + len;
2378+
2379+
/* NOTE: The characters are already validated and are in the [0-9] range */
2380+
assert(off + len <= buflen && "Port number overflow");
2381+
v = 0;
2382+
for (p = buf + off; p < end; p++) {
2383+
v *= 10;
2384+
v += *p - '0';
2385+
2386+
/* Ports have a max value of 2^16 */
2387+
if (v > 0xffff) {
2388+
return 1;
2389+
}
23762390
}
23772391

23782392
u->port = (uint16_t) v;

test.c

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3664,6 +3664,30 @@ test_header_cr_no_lf_error (int req)
36643664
abort();
36653665
}
36663666

3667+
void
3668+
test_no_overflow_parse_url (void)
3669+
{
3670+
int rv;
3671+
struct http_parser_url u;
3672+
3673+
http_parser_url_init(&u);
3674+
rv = http_parser_parse_url("http://example.com:8001", 22, 0, &u);
3675+
3676+
if (rv != 0) {
3677+
fprintf(stderr,
3678+
"\n*** test_no_overflow_parse_url invalid return value=%d\n",
3679+
rv);
3680+
abort();
3681+
}
3682+
3683+
if (u.port != 800) {
3684+
fprintf(stderr,
3685+
"\n*** test_no_overflow_parse_url invalid port number=%d\n",
3686+
u.port);
3687+
abort();
3688+
}
3689+
}
3690+
36673691
void
36683692
test_header_overflow_error (int req)
36693693
{
@@ -4099,6 +4123,7 @@ main (void)
40994123
test_header_nread_value();
41004124

41014125
//// OVERFLOW CONDITIONS
4126+
test_no_overflow_parse_url();
41024127

41034128
test_header_overflow_error(HTTP_REQUEST);
41044129
test_no_overflow_long_body(HTTP_REQUEST, 1000);

0 commit comments

Comments
 (0)