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

Commit

Permalink
http_parser: revert memchr() optimization
Browse files Browse the repository at this point in the history
An optimization was introduced in c6097e1 and 0097de5. The crux of
optimization was to skip all characters in header value until either
of CR or LF. Unfortunately, this optimization comes at cost of
inconsistency in header value validation, which might lead to security
issue due to violated expectations in the user code.

Partially revert the optimization, and add additional check to make
general header value parsing consistent.

Fix: #468
PR-URL: #469
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Harvey Tuch <[email protected]>
  • Loading branch information
indutny committed Apr 9, 2019
1 parent 0d0a24e commit 2a0d106
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 21 deletions.
38 changes: 17 additions & 21 deletions http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -1496,28 +1496,24 @@ size_t http_parser_execute (http_parser *parser,

switch (h_state) {
case h_general:
{
const char* p_cr;
const char* p_lf;
size_t limit = data + len - p;

limit = MIN(limit, max_header_size);

p_cr = (const char*) memchr(p, CR, limit);
p_lf = (const char*) memchr(p, LF, limit);
if (p_cr != NULL) {
if (p_lf != NULL && p_cr >= p_lf)
p = p_lf;
else
p = p_cr;
} else if (UNLIKELY(p_lf != NULL)) {
p = p_lf;
} else {
p = data + len;
{
const char* limit = p + MIN(data + len - p, max_header_size);

for (; p != limit; p++) {
ch = *p;
if (ch == CR || ch == LF) {
--p;
break;
}
if (!lenient && !IS_HEADER_CHAR(ch)) {
SET_ERRNO(HPE_INVALID_HEADER_TOKEN);
goto error;
}
}
if (p == data + len)
--p;
break;
}
--p;
break;
}

case h_connection:
case h_transfer_encoding:
Expand Down
3 changes: 3 additions & 0 deletions test.c
Original file line number Diff line number Diff line change
Expand Up @@ -4316,6 +4316,9 @@ main (void)
test_simple("GET / HTTP/11.1\r\n\r\n", HPE_INVALID_VERSION);
test_simple("GET / HTTP/1.01\r\n\r\n", HPE_INVALID_VERSION);

test_simple("GET / HTTP/1.0\r\nHello: w\1rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);
test_simple("GET / HTTP/1.0\r\nHello: woooo\2rld\r\n\r\n", HPE_INVALID_HEADER_TOKEN);

// Extended characters - see nodejs/test/parallel/test-http-headers-obstext.js
test_simple("GET / HTTP/1.1\r\n"
"Test: Düsseldorf\r\n",
Expand Down

0 comments on commit 2a0d106

Please sign in to comment.