Skip to content

Commit 236222a

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '5.4' into 6.4
* 5.4: Do not read from argv on non-CLI SAPIs [Process] Use %PATH% before %CD% to load the shell on Windows [HttpFoundation] Reject URIs that contain invalid characters [HttpClient] Filter private IPs before connecting when Host == IP
2 parents e750468 + eb79fc2 commit 236222a

File tree

12 files changed

+132
-32
lines changed

12 files changed

+132
-32
lines changed

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,20 @@ public function request(string $method, string $url, array $options = []): Respo
5656

5757
$subnets = $this->subnets;
5858

59-
$options['on_progress'] = function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
59+
$options['on_progress'] = static function (int $dlNow, int $dlSize, array $info) use ($onProgress, $subnets): void {
60+
static $lastUrl = '';
6061
static $lastPrimaryIp = '';
62+
63+
if ($info['url'] !== $lastUrl) {
64+
$host = trim(parse_url($info['url'], PHP_URL_HOST) ?: '', '[]');
65+
66+
if ($host && IpUtils::checkIp($host, $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
67+
throw new TransportException(sprintf('Host "%s" is blocked for "%s".', $host, $info['url']));
68+
}
69+
70+
$lastUrl = $info['url'];
71+
}
72+
6173
if ($info['primary_ip'] !== $lastPrimaryIp) {
6274
if ($info['primary_ip'] && IpUtils::checkIp($info['primary_ip'], $subnets ?? IpUtils::PRIVATE_SUBNETS)) {
6375
throw new TransportException(sprintf('IP "%s" is blocked for "%s".', $info['primary_ip'], $info['url']));

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

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,10 @@ public static function getExcludeData(): array
6565
/**
6666
* @dataProvider getExcludeData
6767
*/
68-
public function testExclude(string $ipAddr, $subnets, bool $mustThrow)
68+
public function testExcludeByIp(string $ipAddr, $subnets, bool $mustThrow)
6969
{
7070
$content = 'foo';
71-
$url = sprintf('http://%s/', 0 < substr_count($ipAddr, ':') ? sprintf('[%s]', $ipAddr) : $ipAddr);
71+
$url = sprintf('http://%s/', strtr($ipAddr, '.:', '--'));
7272

7373
if ($mustThrow) {
7474
$this->expectException(TransportException::class);
@@ -85,6 +85,29 @@ public function testExclude(string $ipAddr, $subnets, bool $mustThrow)
8585
}
8686
}
8787

88+
/**
89+
* @dataProvider getExcludeData
90+
*/
91+
public function testExcludeByHost(string $ipAddr, $subnets, bool $mustThrow)
92+
{
93+
$content = 'foo';
94+
$url = sprintf('http://%s/', str_contains($ipAddr, ':') ? sprintf('[%s]', $ipAddr) : $ipAddr);
95+
96+
if ($mustThrow) {
97+
$this->expectException(TransportException::class);
98+
$this->expectExceptionMessage(sprintf('Host "%s" is blocked for "%s".', $ipAddr, $url));
99+
}
100+
101+
$previousHttpClient = $this->getHttpClientMock($url, $ipAddr, $content);
102+
$client = new NoPrivateNetworkHttpClient($previousHttpClient, $subnets);
103+
$response = $client->request('GET', $url);
104+
105+
if (!$mustThrow) {
106+
$this->assertEquals($content, $response->getContent());
107+
$this->assertEquals(200, $response->getStatusCode());
108+
}
109+
}
110+
88111
public function testCustomOnProgressCallback()
89112
{
90113
$ipAddr = '104.26.14.6';

‎src/Symfony/Component/HttpFoundation/Request.php

Lines changed: 17 additions & 0 deletions
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;
@@ -326,6 +327,8 @@ public static function createFromGlobals(): static
326327
* @param array $files The request files ($_FILES)
327328
* @param array $server The server parameters ($_SERVER)
328329
* @param string|resource|null $content The raw body data
330+
*
331+
* @throws BadRequestException When the URI is invalid
329332
*/
330333
public static function create(string $uri, string $method = 'GET', array $parameters = [], array $cookies = [], array $files = [], array $server = [], $content = null): static
331334
{
@@ -354,6 +357,20 @@ public static function create(string $uri, string $method = 'GET', array $parame
354357
unset($components['fragment']);
355358
}
356359

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

‎src/Symfony/Component/HttpFoundation/Tests/RequestTest.php

Lines changed: 28 additions & 2 deletions
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;
@@ -305,9 +306,34 @@ public function testCreateWithRequestUri()
305306
$this->assertTrue($request->isSecure());
306307

307308
// Fragment should not be included in the URI
308-
$request = Request::create('http://test.com/foo#bar');
309-
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar');
309+
$request = Request::create('http://test.com/foo#bar\\baz');
310+
$request->server->set('REQUEST_URI', 'http://test.com/foo#bar\\baz');
310311
$this->assertEquals('http://test.com/foo', $request->getUri());
312+
313+
$request = Request::create('http://test.com/foo?bar=f\\o');
314+
$this->assertEquals('http://test.com/foo?bar=f%5Co', $request->getUri());
315+
$this->assertEquals('/foo', $request->getPathInfo());
316+
$this->assertEquals('bar=f%5Co', $request->getQueryString());
317+
}
318+
319+
/**
320+
* @testWith ["http://foo.com\\bar"]
321+
* ["\\\\foo.com/bar"]
322+
* ["a\rb"]
323+
* ["a\nb"]
324+
* ["a\tb"]
325+
* ["\u0000foo"]
326+
* ["foo\u0000"]
327+
* [" foo"]
328+
* ["foo "]
329+
* [":"]
330+
*/
331+
public function testCreateWithBadRequestUri(string $uri)
332+
{
333+
$this->expectException(BadRequestException::class);
334+
$this->expectExceptionMessage('Invalid URI');
335+
336+
Request::create($uri);
311337
}
312338

313339
/**

‎src/Symfony/Component/Process/ExecutableFinder.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
*/
2020
class ExecutableFinder
2121
{
22-
private array $suffixes = ['.exe', '.bat', '.cmd', '.com'];
2322
private const CMD_BUILTINS = [
2423
'assoc', 'break', 'call', 'cd', 'chdir', 'cls', 'color', 'copy', 'date',
2524
'del', 'dir', 'echo', 'endlocal', 'erase', 'exit', 'for', 'ftype', 'goto',
@@ -28,6 +27,8 @@ class ExecutableFinder
2827
'setlocal', 'shift', 'start', 'time', 'title', 'type', 'ver', 'vol',
2928
];
3029

30+
private array $suffixes = [];
31+
3132
/**
3233
* Replaces default suffixes of executable.
3334
*
@@ -67,11 +68,13 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
6768
$extraDirs
6869
);
6970

70-
$suffixes = [''];
71+
$suffixes = [];
7172
if ('\\' === \DIRECTORY_SEPARATOR) {
7273
$pathExt = getenv('PATHEXT');
73-
$suffixes = array_merge($pathExt ? explode(\PATH_SEPARATOR, $pathExt) : $this->suffixes, $suffixes);
74+
$suffixes = $this->suffixes;
75+
$suffixes = array_merge($suffixes, $pathExt ? explode(\PATH_SEPARATOR, $pathExt) : ['.exe', '.bat', '.cmd', '.com']);
7476
}
77+
$suffixes = '' !== pathinfo($name, PATHINFO_EXTENSION) ? array_merge([''], $suffixes) : array_merge($suffixes, ['']);
7578
foreach ($suffixes as $suffix) {
7679
foreach ($dirs as $dir) {
7780
if ('' === $dir) {
@@ -87,12 +90,11 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
8790
}
8891
}
8992

90-
if (!\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
93+
if ('\\' === \DIRECTORY_SEPARATOR || !\function_exists('exec') || \strlen($name) !== strcspn($name, '/'.\DIRECTORY_SEPARATOR)) {
9194
return $default;
9295
}
9396

94-
$command = '\\' === \DIRECTORY_SEPARATOR ? 'where %s 2> NUL' : 'command -v -- %s';
95-
$execResult = exec(\sprintf($command, escapeshellarg($name)));
97+
$execResult = exec('command -v -- '.escapeshellarg($name));
9698

9799
if (($executablePath = substr($execResult, 0, strpos($execResult, \PHP_EOL) ?: null)) && @is_executable($executablePath)) {
98100
return $executablePath;

‎src/Symfony/Component/Process/PhpExecutableFinder.php

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,8 @@ public function __construct()
3232
public function find(bool $includeArgs = true): string|false
3333
{
3434
if ($php = getenv('PHP_BINARY')) {
35-
if (!is_executable($php)) {
36-
if (!\function_exists('exec') || \strlen($php) !== strcspn($php, '/'.\DIRECTORY_SEPARATOR)) {
37-
return false;
38-
}
39-
40-
$command = '\\' === \DIRECTORY_SEPARATOR ? 'where %s 2> NUL' : 'command -v -- %s';
41-
$execResult = exec(\sprintf($command, escapeshellarg($php)));
42-
if (!$php = substr($execResult, 0, strpos($execResult, \PHP_EOL) ?: null)) {
43-
return false;
44-
}
45-
if (!is_executable($php)) {
46-
return false;
47-
}
35+
if (!is_executable($php) && !$php = $this->executableFinder->find($php)) {
36+
return false;
4837
}
4938

5039
if (@is_dir($php)) {

‎src/Symfony/Component/Process/Process.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1536,7 +1536,14 @@ function ($m) use (&$env, $uid) {
15361536
$cmd
15371537
);
15381538

1539-
$cmd = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
1539+
static $comSpec;
1540+
1541+
if (!$comSpec && $comSpec = (new ExecutableFinder())->find('cmd.exe')) {
1542+
// Escape according to CommandLineToArgvW rules
1543+
$comSpec = '"'.preg_replace('{(\\\\*+)"}', '$1$1\"', $comSpec) .'"';
1544+
}
1545+
1546+
$cmd = ($comSpec ?? 'cmd').' /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $cmd).')';
15401547
foreach ($this->processPipes->getFiles() as $offset => $filename) {
15411548
$cmd .= ' '.$offset.'>"'.$filename.'"';
15421549
}

‎src/Symfony/Component/Runtime/SymfonyRuntime.php

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ public function __construct(array $options = [])
9595

9696
if (isset($options['env'])) {
9797
$_SERVER[$envKey] = $options['env'];
98-
} elseif (isset($_SERVER['argv']) && class_exists(ArgvInput::class)) {
98+
} elseif (empty($_GET) && isset($_SERVER['argv']) && class_exists(ArgvInput::class)) {
9999
$this->options = $options;
100100
$this->getInput();
101101
}
@@ -203,6 +203,10 @@ protected static function register(GenericRuntime $runtime): GenericRuntime
203203

204204
private function getInput(): ArgvInput
205205
{
206+
if (!empty($_GET) && filter_var(ini_get('register_argc_argv'), \FILTER_VALIDATE_BOOL)) {
207+
throw new \Exception('CLI applications cannot be run safely on non-CLI SAPIs with register_argc_argv=On.');
208+
}
209+
206210
if (isset($this->input)) {
207211
return $this->input;
208212
}

‎src/Symfony/Component/Runtime/Tests/phpt/kernel-loop.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ require __DIR__.'/kernel-loop.php';
1111

1212
?>
1313
--EXPECTF--
14-
OK Kernel foo_bar
15-
OK Kernel foo_bar
14+
OK Kernel (env=dev) foo_bar
15+
OK Kernel (env=dev) foo_bar
1616
0

‎src/Symfony/Component/Runtime/Tests/phpt/kernel.php

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,19 @@
1717

1818
class TestKernel implements HttpKernelInterface
1919
{
20+
private string $env;
2021
private string $var;
2122

22-
public function __construct(string $var)
23+
public function __construct(string $env, string $var)
2324
{
25+
$this->env = $env;
2426
$this->var = $var;
2527
}
2628

2729
public function handle(Request $request, $type = self::MAIN_REQUEST, $catch = true): Response
2830
{
29-
return new Response('OK Kernel '.$this->var);
31+
return new Response('OK Kernel (env='.$this->env.') '.$this->var);
3032
}
3133
}
3234

33-
return fn (array $context) => new TestKernel($context['SOME_VAR']);
35+
return fn (array $context) => new TestKernel($context['APP_ENV'], $context['SOME_VAR']);

0 commit comments

Comments
 (0)