Skip to content

Commit

Permalink
security: strict match regex start/end
Browse files Browse the repository at this point in the history
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 <matthew@weierophinney.net>
  • Loading branch information
weierophinney committed Apr 12, 2023
1 parent c33a4e7 commit 238a44a
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/HeaderSecurity.php
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public static function assertValidName($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
Expand Down
28 changes: 28 additions & 0 deletions test/HeaderSecurityTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
];
}

Expand Down Expand Up @@ -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],
];
}

Expand Down Expand Up @@ -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"],
];
}

Expand All @@ -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);
}
}
4 changes: 4 additions & 0 deletions test/MessageTraitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,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"],
Expand All @@ -290,6 +292,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"],
];
}

Expand Down

0 comments on commit 238a44a

Please sign in to comment.