From 61f85d14ee4444755e0771495b97af11162448dd Mon Sep 17 00:00:00 2001 From: Glenn Strauss Date: Sat, 28 Sep 2019 19:21:56 -0400 Subject: [PATCH] [core] reject WS following header field-name (fixes #2985) reject whitespace following request header field-name and before colon Such whitespace is forbidden in RFC 7230 Section 3.2.4. strict header parsing is enabled by default in lighttpd. However, if explicitly disabled in lighttpd.conf, lighttpd will continue to accept (and re-format) such field-names before passing to any backend. UNSAFE: server.http-parseopts = ( "header-strict" => "disable" ) This is NOT RECOMMENDED since doing so disables other protections provided by lighttpd strict http header parsing. (thx fedormixalich) x-ref: stricter request header parsing https://redmine.lighttpd.net/issues/2985 --- src/request.c | 13 +++++++++++++ src/t/test_request.c | 5 +---- tests/request.t | 12 +----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/request.c b/src/request.c index b72bb974..64b2ba45 100644 --- a/src/request.c +++ b/src/request.c @@ -723,6 +723,19 @@ int http_request_parse(server *srv, connection *con, buffer *hdrs) { switch(*cur) { case ' ': case '\t': + /* RFC7230 Hypertext Transfer Protocol (HTTP/1.1): Message Syntax and Routing + * 3.2.4. Field Parsing + * [...] + * No whitespace is allowed between the header field-name and colon. In + * the past, differences in the handling of such whitespace have led to + * security vulnerabilities in request routing and response handling. A + * server MUST reject any received request message that contains + * whitespace between a header field-name and colon with a response code + * of 400 (Bad Request). A proxy MUST remove any such whitespace from a + * response message before forwarding the message downstream. + */ + if (http_header_strict) + return http_request_header_line_invalid(srv, 400, "invalid whitespace between field-name and colon -> 400"); /* skip every thing up to the : */ do { ++cur; } while (*cur == ' ' || *cur == '\t'); if (*cur != ':') { diff --git a/src/t/test_request.c b/src/t/test_request.c index e001fb6a..1387565e 100644 --- a/src/t/test_request.c +++ b/src/t/test_request.c @@ -310,14 +310,11 @@ static void test_request_http_request_parse(server *srv, connection *con) assert(buffer_is_equal_string(con->request.uri, CONST_STR_LEN("/"))); - run_http_request_parse(srv, con, __LINE__, 0, + run_http_request_parse(srv, con, __LINE__, 400, "whitespace after key", CONST_STR_LEN("GET / HTTP/1.0\r\n" "ABC : foo\r\n" "\r\n")); - ds = (data_string *) - array_get_element_klen(con->request.headers, CONST_STR_LEN("ABC")); - assert(ds && buffer_is_equal_string(ds->value, CONST_STR_LEN("foo"))); run_http_request_parse(srv, con, __LINE__, 400, "whitespace within key", diff --git a/tests/request.t b/tests/request.t index 96ef077b..aa1cace0 100755 --- a/tests/request.t +++ b/tests/request.t @@ -8,7 +8,7 @@ BEGIN { use strict; use IO::Socket; -use Test::More tests => 52; +use Test::More tests => 51; use LightyTest; my $tf = LightyTest->new(); @@ -503,16 +503,6 @@ $t->{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.0', 'HTTP-Status' => 403 } ]; ok($tf->handle_http($t) == 0, 'static file with forbidden pathinfo'); -print "\nConnection header\n"; -$t->{REQUEST} = ( <{RESPONSE} = [ { 'HTTP-Protocol' => 'HTTP/1.1', 'HTTP-Status' => 200, 'HTTP-Content' => '12345'."\n", 'Content-Type' => 'text/plain', 'Connection' => 'close' } ]; -ok($tf->handle_http($t) == 0, 'Connection-header, spaces before ":"'); - $t->{REQUEST} = ( <