Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/Protocols/Http.php
Original file line number Diff line number Diff line change
Expand Up @@ -134,20 +134,20 @@ public static function input(string $buffer, TcpConnection $connection): int
// Missing the host header or value for HTTP/1.1 requests (case-insensitive; line-start must be "\r\n" to avoid matching "x-Host").
. '(?:(?=[\s\S]*\r\nHost[ \t]*:[ \t]*(?:[^\r\n]+)?\r\n))?'
// Optional: capture Content-Length value (must be at \A to scan entire header)
. '(?:(?=[\s\S]*\r\nContent-Length[ \t]*:[ \t]*(?<length>\d+)[ \t]*\r\n))?'
. '(?:(?=[\s\S]*\r\nContent-Length:[ \t]*(?<length>\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
. '(?:(?-i:GET|POST|OPTIONS|HEAD|DELETE|PUT|PATCH) )+(?:/[^\x00-\x20\x7f]*)+(?: (?-i:HTTP)/1.[0-9])\r\n'
// Flag case-insensitive
. '~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);
Expand Down Expand Up @@ -185,9 +185,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)) {
Expand Down Expand Up @@ -290,7 +290,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;
Expand Down
8 changes: 8 additions & 0 deletions tests/Unit/Protocols/HttpTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,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.
'lowercase method is not allowed' => [
"get / HTTP/1.1\r\n\r\n",
],
Expand Down
Loading