Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 352786c

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
bug #58776 [DependencyInjection][HttpClient][Routing] Reject URIs that contain invalid characters (nicolas-grekas)
This PR was merged into the 7.2 branch. Discussion ---------- [DependencyInjection][HttpClient][Routing] Reject URIs that contain invalid characters | Q | A | ------------- | --- | Branch? | 7.2 | Bug fix? | no | New feature? | no | Deprecations? | no | Issues | - | License | MIT The behavior of `parse_url()` doesn't match how browsers parse URLs. This PR ensures we won't accept URLs that are invalid per https://url.spec.whatwg.org/: - URLs that contain a backslash - or start/end with a control-char or a space - or contain a CR/LF/TAB character Commits ------- d73003e [DependencyInjection][Routing][HttpClient] Reject URIs that contain invalid characters
2 parents 2c9bcaa + d73003e commit 352786c
Copy full SHA for 352786c

File tree

6 files changed

+86
-1
lines changed
Filter options

6 files changed

+86
-1
lines changed

‎src/Symfony/Component/DependencyInjection/EnvVarProcessor.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/EnvVarProcessor.php
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,12 @@ public function getEnv(string $prefix, string $name, \Closure $getEnv): mixed
310310
if (!isset($params['scheme'], $params['host'])) {
311311
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": scheme and host expected.', $name));
312312
}
313+
if (('\\' !== \DIRECTORY_SEPARATOR || 'file' !== $params['scheme']) && false !== ($i = strpos($env, '\\')) && $i < strcspn($env, '?#')) {
314+
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": backslashes are not allowed.', $name));
315+
}
316+
if (\ord($env[0]) <= 32 || \ord($env[-1]) <= 32 || \strlen($env) !== strcspn($env, "\r\n\t")) {
317+
throw new RuntimeException(\sprintf('Invalid URL in env var "%s": leading/trailing ASCII control characters or whitespaces are not allowed.', $name));
318+
}
313319
$params += [
314320
'port' => null,
315321
'user' => null,

‎src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/DependencyInjection/Tests/EnvVarProcessorTest.php
+21Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -996,6 +996,27 @@ public static function provideGetEnvUrlPath()
996996
];
997997
}
998998

999+
/**
1000+
* @testWith ["http://foo.com\\bar"]
1001+
* ["\\\\foo.com/bar"]
1002+
* ["a\rb"]
1003+
* ["a\nb"]
1004+
* ["a\tb"]
1005+
* ["\u0000foo"]
1006+
* ["foo\u0000"]
1007+
* [" foo"]
1008+
* ["foo "]
1009+
* [":"]
1010+
*/
1011+
public function testGetEnvBadUrl(string $url)
1012+
{
1013+
$this->expectException(RuntimeException::class);
1014+
1015+
(new EnvVarProcessor(new Container()))->getEnv('url', 'foo', static function () use ($url): string {
1016+
return $url;
1017+
});
1018+
}
1019+
9991020
/**
10001021
* @testWith ["", "string"]
10011022
* [null, ""]

‎src/Symfony/Component/HttpClient/HttpClientTrait.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/HttpClientTrait.php
+10Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -625,6 +625,16 @@ private static function resolveUrl(array $url, ?array $base, array $queryDefault
625625
*/
626626
private static function parseUrl(string $url, array $query = [], array $allowedSchemes = ['http' => 80, 'https' => 443]): array
627627
{
628+
if (false !== ($i = strpos($url, '\\')) && $i < strcspn($url, '?#')) {
629+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": backslashes are not allowed.', $url));
630+
}
631+
if (\strlen($url) !== strcspn($url, "\r\n\t")) {
632+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": CR/LF/TAB characters are not allowed.', $url));
633+
}
634+
if ('' !== $url && (\ord($url[0]) <= 32 || \ord($url[-1]) <= 32)) {
635+
throw new InvalidArgumentException(\sprintf('Malformed URL "%s": leading/trailing ASCII control characters or spaces are not allowed.', $url));
636+
}
637+
628638
if (false === $parts = parse_url($url)) {
629639
if ('/' !== ($url[0] ?? '') || false === $parts = parse_url($url.'#')) {
630640
throw new InvalidArgumentException(\sprintf('Malformed URL "%s".', $url));

‎src/Symfony/Component/HttpClient/Tests/HttpClientTraitTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/HttpClient/Tests/HttpClientTraitTest.php
+20-1Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,13 +239,32 @@ public function testResolveUrlWithoutScheme()
239239
self::resolveUrl(self::parseUrl('localhost:8080'), null);
240240
}
241241

242-
public function testResolveBaseUrlWitoutScheme()
242+
public function testResolveBaseUrlWithoutScheme()
243243
{
244244
$this->expectException(InvalidArgumentException::class);
245245
$this->expectExceptionMessage('Invalid URL: scheme is missing in "//localhost:8081". Did you forget to add "http(s)://"?');
246246
self::resolveUrl(self::parseUrl('/foo'), self::parseUrl('localhost:8081'));
247247
}
248248

249+
/**
250+
* @testWith ["http://foo.com\\bar"]
251+
* ["\\\\foo.com/bar"]
252+
* ["a\rb"]
253+
* ["a\nb"]
254+
* ["a\tb"]
255+
* ["\u0000foo"]
256+
* ["foo\u0000"]
257+
* [" foo"]
258+
* ["foo "]
259+
* [":"]
260+
*/
261+
public function testParseMalformedUrl(string $url)
262+
{
263+
$this->expectException(InvalidArgumentException::class);
264+
$this->expectExceptionMessage('Malformed URL');
265+
self::parseUrl($url);
266+
}
267+
249268
/**
250269
* @dataProvider provideParseUrl
251270
*/

‎src/Symfony/Component/Routing/RequestContext.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/RequestContext.php
+7Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ public function __construct(string $baseUrl = '', string $method = 'GET', string
4747

4848
public static function fromUri(string $uri, string $host = 'localhost', string $scheme = 'http', int $httpPort = 80, int $httpsPort = 443): self
4949
{
50+
if (false !== ($i = strpos($uri, '\\')) && $i < strcspn($uri, '?#')) {
51+
$uri = '';
52+
}
53+
if ('' !== $uri && (\ord($uri[0]) <= 32 || \ord($uri[-1]) <= 32 || \strlen($uri) !== strcspn($uri, "\r\n\t"))) {
54+
$uri = '';
55+
}
56+
5057
$uri = parse_url($uri);
5158
$scheme = $uri['scheme'] ?? $scheme;
5259
$host = $uri['host'] ?? $host;

‎src/Symfony/Component/Routing/Tests/RequestContextTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Routing/Tests/RequestContextTest.php
+22Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,28 @@ public function testFromUriBeingEmpty()
8585
$this->assertSame('/', $requestContext->getPathInfo());
8686
}
8787

88+
/**
89+
* @testWith ["http://foo.com\\bar"]
90+
* ["\\\\foo.com/bar"]
91+
* ["a\rb"]
92+
* ["a\nb"]
93+
* ["a\tb"]
94+
* ["\u0000foo"]
95+
* ["foo\u0000"]
96+
* [" foo"]
97+
* ["foo "]
98+
* [":"]
99+
*/
100+
public function testFromBadUri(string $uri)
101+
{
102+
$context = RequestContext::fromUri($uri);
103+
104+
$this->assertSame('http', $context->getScheme());
105+
$this->assertSame('localhost', $context->getHost());
106+
$this->assertSame('', $context->getBaseUrl());
107+
$this->assertSame('/', $context->getPathInfo());
108+
}
109+
88110
public function testFromRequest()
89111
{
90112
$request = Request::create('https://test.com:444/foo?bar=baz');

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.