From 70ccc2ac176ce358d1722bb0769c17d65dc72819 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 13 Apr 2023 10:09:20 -0500 Subject: [PATCH] security: strict match regex start/end Adds the `D` modifier to the regex validating a header name or value, to ensure it does not ignore a newline at the start or end of the string. Patch developed by @TimWolla Signed-off-by: Matthew Weier O'Phinney --- src/HeaderSecurity.php | 2 +- test/HeaderSecurityTest.php | 28 ++++++++++++++++++++++++++++ test/MessageTraitTest.php | 4 ++++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/src/HeaderSecurity.php b/src/HeaderSecurity.php index 08a5e3c3..d12486ae 100644 --- a/src/HeaderSecurity.php +++ b/src/HeaderSecurity.php @@ -154,7 +154,7 @@ public static function assertValidName(mixed $name): void is_object($name) ? $name::class : gettype($name) )); } - if (! preg_match('/^[a-zA-Z0-9\'`#$%&*+.^_|~!-]+$/', $name)) { + if (! preg_match('/^[a-zA-Z0-9\'`#$%&*+.^_|~!-]+$/D', $name)) { throw new Exception\InvalidArgumentException(sprintf( '"%s" is not valid header name', $name diff --git a/test/HeaderSecurityTest.php b/test/HeaderSecurityTest.php index 100d1bf4..ca7c49d9 100644 --- a/test/HeaderSecurityTest.php +++ b/test/HeaderSecurityTest.php @@ -29,6 +29,7 @@ public function getFilterValues(): array ["This is a \r\r\n test", "This is a \r\n test"], ["This is a \r\n\r\ntest", "This is a test"], ["This is a \r\n\n\r\n test", "This is a \r\n test"], + ["This is a test\n", "This is a test"], ]; } @@ -61,6 +62,7 @@ public function validateValues(): array ["This is a \xFF test", false], ["This is a \x7F test", false], ["This is a \x7E test", true], + ["This is a test\n", false], ]; } @@ -88,6 +90,7 @@ public function assertValues(): array ["This is a \r\r\n test"], ["This is a \r\n\r\ntest"], ["This is a \r\n\n\r\n test"], + ["This is a test\n"], ]; } @@ -102,4 +105,29 @@ public function testAssertValidRaisesExceptionForInvalidValue(string $value): vo HeaderSecurity::assertValid($value); } + + /** @return non-empty-list */ + public function assertNames(): array + { + return [ + ["test\n"], + ["\ntest"], + ["foo\r\n bar"], + ["f\x00o"], + ["foo bar"], + [":foo"], + ["foo:"], + ]; + } + + /** + * @dataProvider assertNames + * @param non-empty-string $value + */ + public function testAssertValidNameRaisesExceptionForInvalidName(string $value): void + { + $this->expectException(InvalidArgumentException::class); + + HeaderSecurity::assertValidName($value); + } } diff --git a/test/MessageTraitTest.php b/test/MessageTraitTest.php index 4c636926..6f21b6a7 100644 --- a/test/MessageTraitTest.php +++ b/test/MessageTraitTest.php @@ -278,6 +278,8 @@ public function headersWithInjectionVectors(): array 'name-with-lf' => ["X-Foo\n-Bar", 'value'], 'name-with-crlf' => ["X-Foo\r\n-Bar", 'value'], 'name-with-2crlf' => ["X-Foo\r\n\r\n-Bar", 'value'], + 'name-with-trailing-lf' => ["X-Foo-Bar\n", 'value'], + 'name-with-leading-lf' => ["\nX-Foo-Bar", 'value'], 'value-with-cr' => ['X-Foo-Bar', "value\rinjection"], 'value-with-lf' => ['X-Foo-Bar', "value\ninjection"], 'value-with-crlf' => ['X-Foo-Bar', "value\r\ninjection"], @@ -286,6 +288,8 @@ public function headersWithInjectionVectors(): array 'array-value-with-lf' => ['X-Foo-Bar', ["value\ninjection"]], 'array-value-with-crlf' => ['X-Foo-Bar', ["value\r\ninjection"]], 'array-value-with-2crlf' => ['X-Foo-Bar', ["value\r\n\r\ninjection"]], + 'value-with-trailing-lf' => ['X-Foo-Bar', "value\n"], + 'value-with-leading-lf' => ['X-Foo-Bar', "\nvalue"], ]; }