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 <[email protected]>
laminas · Apr 13, 2023 · 70ccc2a · 70ccc2a
1 parent a7a7f72
commit 70ccc2a
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 1 deletion.
diff --git 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
@@ -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<array{non-empty-string}> */
+    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
@@ -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"],
         ];
     }