diff --git a/src/Protocols/Http.php b/src/Protocols/Http.php index 3d100e9e..e4ae4e9a 100644 --- a/src/Protocols/Http.php +++ b/src/Protocols/Http.php @@ -132,18 +132,18 @@ public static function input(string $buffer, TcpConnection $connection): int // The pattern uses case-insensitive modifier (~i) for header name matching. $headerValidatePattern = '~\A' // Optional: capture Content-Length value (must be at \A to scan entire header) - . '(?:(?=[\s\S]*\r\nContent-Length[ \t]*:[ \t]*(\d+)[ \t]*\r\n))?' + . '(?:(?=[\s\S]*\r\nContent-Length:[ \t]*(\d+)[ \t]*\r\n))?' // Disallow Transfer-Encoding header - . '(?![\s\S]*\r\nTransfer-Encoding[ \t]*:)' + . '(?![\s\S]*\r\nTransfer-Encoding:)' // If Content-Length header exists, its value must be pure digits + optional OWS - . '(?![\s\S]*\r\nContent-Length[ \t]*:(?![ \t]*\d+[ \t]*\r\n)[^\r]*\r\n)' + . '(?![\s\S]*\r\nContent-Length:(?![ \t]*\d+[ \t]*\r\n)[^\r]*\r\n)' // Disallow duplicate Content-Length headers (adjacent or separated) - . '(?![\s\S]*\r\nContent-Length[ \t]*:[^\r\n]*\r\n(?:[\s\S]*?\r\n)?Content-Length[ \t]*:)' + . '(?![\s\S]*\r\nContent-Length:[^\r\n]*\r\n(?:[\s\S]*?\r\n)?Content-Length:)' // Match request line: METHOD SP request-target SP HTTP-version CRLF . '(?:GET|POST|OPTIONS|HEAD|DELETE|PUT|PATCH) +\/[^\x00-\x20\x7f]* +HTTP\/1\.[01]\r\n~i'; if (!preg_match($headerValidatePattern, $header, $matches)) { - if (preg_match('~\r\nTransfer-Encoding[ \t]*:~i', $header)) { + if (preg_match('~\r\nTransfer-Encoding:~i', $header)) { return static::inputChunked($buffer, $connection, $header, $length); } $connection->end(static::HTTP_400, true); @@ -181,9 +181,9 @@ public static function input(string $buffer, TcpConnection $connection): int protected static function inputChunked(string $buffer, TcpConnection $connection, string $header, int $headerLength): int { $pattern = '~\A' - . '(?![\s\S]*\r\nContent-Length[ \t]*:)' - . '(?![\s\S]*\r\nTransfer-Encoding[ \t]*:[\s\S]*\r\nTransfer-Encoding[ \t]*:)' - . '(?=[\s\S]*\r\nTransfer-Encoding[ \t]*:[ \t]*chunked[ \t]*\r\n)' + . '(?![\s\S]*\r\nContent-Length:)' + . '(?![\s\S]*\r\nTransfer-Encoding:[\s\S]*\r\nTransfer-Encoding[ \t]*:)' + . '(?=[\s\S]*\r\nTransfer-Encoding:[ \t]*chunked[ \t]*\r\n)' . '(?:GET|POST|OPTIONS|HEAD|DELETE|PUT|PATCH) +\/[^\x00-\x20\x7f]* +HTTP\/1\.[01]\r\n~i'; if (!preg_match($pattern, $header)) { @@ -286,7 +286,7 @@ public static function decode(string $buffer, TcpConnection $connection): mixed */ protected static function decodeChunked(string $buffer, int $headerEnd): array { - $header = preg_replace('~\r\nTransfer-Encoding[ \t]*:[^\r]*~i', '', substr($buffer, 0, $headerEnd), 1); + $header = preg_replace('~\r\nTransfer-Encoding:[^\r]*~i', '', substr($buffer, 0, $headerEnd), 1); $body = ''; $trailers = []; $pos = $headerEnd + 4; diff --git a/tests/Unit/Protocols/HttpTest.php b/tests/Unit/Protocols/HttpTest.php index b8d8bd3f..8977b429 100644 --- a/tests/Unit/Protocols/HttpTest.php +++ b/tests/Unit/Protocols/HttpTest.php @@ -158,6 +158,14 @@ 'CRLF injection attempt in request-target' => [ "GET /foo\r\nX: y HTTP/1.1\r\n\r\n", ], + // OWS is not allowed between the field name and colon because it would interfere with header parsing and validation regexes. (OWS is allowed after the colon and around the value, but not before the colon.) + 'OWS between Content-Length :' => [ + "GET / HTTP/1.1\r\n\Content-Length : 0\r\n\r\n", + ], + 'OWS between Transfer-Encoding :' => [ + "GET / HTTP/1.1\r\n\Transfer-Encoding : chunked\r\n\r\n", + ], + // We need more tests about leading OWS before other headers to ensure it is properly rejected and does not interfere with header parsing and validation. For example, if leading OWS before Content-Length is not rejected, it could allow smuggling of a second Content-Length header that would be parsed by the regexes as the only Content-Length header, bypassing the duplicate Content-Length header check and allowing a request with multiple Content-Length headers, which is forbidden by the HTTP spec and can cause security issues. ]); it('rejects Transfer-Encoding and bad/duplicate Content-Length in ::input', function (string $buffer, ?string $expectedCloseContains = '400 Bad Request') {