Skip to content

Commit 168b77c

Browse files
security #cve-2024-50345 [HttpFoundation] Reject URIs that contain invalid characters (nicolas-grekas)
This PR was merged into the 5.4 branch.
2 parents 38bd9bc + 32310ff commit 168b77c

File tree

2 files changed

+45
-2
lines changed

2 files changed

+45
-2
lines changed

Request.php

+17
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\HttpFoundation;
1313

14+
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
1415
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1516
use Symfony\Component\HttpFoundation\Exception\JsonException;
1617
use Symfony\Component\HttpFoundation\Exception\SessionNotFoundException;
@@ -333,6 +334,8 @@ public static function createFromGlobals()
333334
* @param string|resource|null $content The raw body data
334335
*
335336
* @return static
337+
*
338+
* @throws BadRequestException When the URI is invalid
336339
*/
337340
public static function create(string $uri, string $method = 'GET', array $parameters = [], array $cookies = [], array $files = [], array $server = [], $content = null)
338341
{
@@ -360,6 +363,20 @@ public static function create(string $uri, string $method = 'GET', array $parame
360363
unset($components['fragment']);
361364
}
362365

366+
if (false === $components) {
367+
throw new BadRequestException('Invalid URI.');
368+
}
369+
370+
if (false !== ($i = strpos($uri, '\\')) && $i < strcspn($uri, '?#')) {
371+
throw new BadRequestException('Invalid URI: A URI cannot contain a backslash.');
372+
}
373+
if (\strlen($uri) !== strcspn($uri, "\r\n\t")) {
374+
throw new BadRequestException('Invalid URI: A URI cannot contain CR/LF/TAB characters.');
375+
}
376+
if ('' !== $uri && (\ord($uri[0]) <= 32 || \ord($uri[-1]) <= 32)) {
377+
throw new BadRequestException('Invalid URI: A URI must not start nor end with ASCII control characters or spaces.');
378+
}
379+
363380
if (isset($components['host'])) {
364381
$server['SERVER_NAME'] = $components['host'];
365382
$server['HTTP_HOST'] = $components['host'];

Tests/RequestTest.php

+28-2
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use PHPUnit\Framework\TestCase;
1515
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
16+
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
1617
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
1718
use Symfony\Component\HttpFoundation\Exception\JsonException;
1819
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
@@ -289,9 +290,34 @@ public function testCreateWithRequestUri()
289290
$this->assertTrue($request->isSecure());
290291

291292
// Fragment should not be included in the URI
292-
$request = Request::create('http://test.com/foo#bar');
293-
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar');
293+
$request = Request::create('http://test.com/foo#bar\\baz');
294+
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar\\baz');
294295
$this->assertEquals('http://test.com/foo', $request->getUri());
296+
297+
$request = Request::create('http://test.com/foo?bar=f\\o');
298+
$this->assertEquals('http://test.com/foo?bar=f%5Co', $request->getUri());
299+
$this->assertEquals('/foo', $request->getPathInfo());
300+
$this->assertEquals('bar=f%5Co', $request->getQueryString());
301+
}
302+
303+
/**
304+
* @testWith ["http://foo.com\\bar"]
305+
* ["\\\\foo.com/bar"]
306+
* ["a\rb"]
307+
* ["a\nb"]
308+
* ["a\tb"]
309+
* ["\u0000foo"]
310+
* ["foo\u0000"]
311+
* [" foo"]
312+
* ["foo "]
313+
* [":"]
314+
*/
315+
public function testCreateWithBadRequestUri(string $uri)
316+
{
317+
$this->expectException(BadRequestException::class);
318+
$this->expectExceptionMessage('Invalid URI');
319+
320+
Request::create($uri);
295321
}
296322

297323
/**

0 commit comments

Comments
 (0)