From 4fea29e910391b1883de5bf6e84b50f6900355fb Mon Sep 17 00:00:00 2001 From: Rob Allen Date: Tue, 11 Apr 2023 18:47:28 +0100 Subject: [PATCH] Security fix: Reject newlines at end of header names If a header name has multiple newlines at the end of its name, it causes the remaining headers to be pushed down into the body. Mitigate this using the `D` regex modifier. Thanks to Graham Campbell for the report and fix. --- src/Headers.php | 4 +-- tests/HeadersTest.php | 76 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/src/Headers.php b/src/Headers.php index 038e792..e413147 100644 --- a/src/Headers.php +++ b/src/Headers.php @@ -266,7 +266,7 @@ protected function validateHeader($name, $value): void */ protected function validateHeaderName($name): void { - if (!is_string($name) || preg_match("@^[!#$%&'*+.^_`|~0-9A-Za-z-]+$@", $name) !== 1) { + if (!is_string($name) || preg_match("@^[!#$%&'*+.^_`|~0-9A-Za-z-]+$@D", $name) !== 1) { throw new InvalidArgumentException('Header name must be an RFC 7230 compatible string.'); } } @@ -286,7 +286,7 @@ protected function validateHeaderValue($value): void ); } - $pattern = "@^[ \t\x21-\x7E\x80-\xFF]*$@"; + $pattern = "@^[ \t\x21-\x7E\x80-\xFF]*$@D"; foreach ($items as $item) { $hasInvalidType = !is_numeric($item) && !is_string($item); $rejected = $hasInvalidType || preg_match($pattern, (string) $item) !== 1; diff --git a/tests/HeadersTest.php b/tests/HeadersTest.php index 02d643a..0642058 100644 --- a/tests/HeadersTest.php +++ b/tests/HeadersTest.php @@ -209,4 +209,80 @@ public function testParseAuthorizationHeader() $headers = new Headers([], ['PHP_AUTH_DIGEST' => 'digest']); $this->assertEquals(['digest'], $headers->getHeader('Authorization')); } + + /** + * @dataProvider provideInvalidHeaderNames + */ + public function testWithInvalidHeaderName($headerName): void + { + $headers = new Headers(); + + $this->expectException(\InvalidArgumentException::class); + + $headers->setHeader($headerName, 'foo'); + } + + public static function provideInvalidHeaderNames(): array + { + return [ + [[]], + [false], + [new \stdClass()], + ["Content-Type\r\n\r\n"], + ["Content-Type\r\n"], + ["Content-Type\n"], + ["\r\nContent-Type"], + ["\nContent-Type"], + ["\n"], + ["\r\n"], + ["\t"], + ]; + } + + /** + * @dataProvider provideInvalidHeaderValues + */ + public function testSetInvalidHeaderValue($headerValue) + { + $headers = new Headers(); + + $this->expectException(\InvalidArgumentException::class); + + $headers->setHeader('Content-Type', $headerValue); + } + + public static function provideInvalidHeaderValues(): array + { + // Explicit tests for newlines as the most common exploit vector. + $tests = [ + ["new\nline"], + ["new\r\nline"], + ["new\rline"], + ["new\r\n line"], + ["newline\n"], + ["\nnewline"], + ["newline\r\n"], + ["\n\rnewline"], + ]; + + for ($i = 0; $i <= 0xff; $i++) { + if (\chr($i) == "\t") { + continue; + } + if (\chr($i) == " ") { + continue; + } + if ($i >= 0x21 && $i <= 0x7e) { + continue; + } + if ($i >= 0x80) { + continue; + } + + $tests[] = ["foo" . \chr($i) . "bar"]; + $tests[] = ["foo" . \chr($i)]; + } + + return $tests; + } }