From 2a0d1065d4f68cf622db2ffd22dc8022c0239fa4 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 5 Apr 2019 16:49:14 -0400 Subject: [PATCH] http_parser: revert `memchr()` optimization An optimization was introduced in c6097e1d and 0097de58. 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: https://github.com/nodejs/http-parser/pull/469 Reviewed-By: Sam Roberts Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: Harvey Tuch --- http_parser.c | 38 +++++++++++++++++--------------------- test.c | 3 +++ 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/http_parser.c b/http_parser.c index e2fc5d2..2ea228e 100644 --- a/http_parser.c +++ b/http_parser.c @@ -1496,28 +1496,24 @@ reexecute: 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: diff --git a/test.c b/test.c index c3fddd5..0140a18 100644 --- a/test.c +++ b/test.c @@ -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",