Skip to content

[Process] do not search in $PATH entries not allowed by open_basedir #58008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Process] do not search in $PATH entries not allowed by open_basedir #58008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/Symfony/Component/Process/ExecutableFinder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,14 @@ public function addSuffix(string $suffix)
*/
public function find(string $name, ?string $default = null, array $extraDirs = [])
{
$dirs = array_merge(
explode(\PATH_SEPARATOR, getenv('PATH') ?: getenv('Path')),
$extraDirs
);

if (\ini_get('open_basedir')) {
$searchPath = array_merge(explode(\PATH_SEPARATOR, \ini_get('open_basedir')), $extraDirs);
$dirs = [];

Comment on lines 56 to +58
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the new logic $extraDirs do not have to be searched here. Imho #57954 is the correct variant.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#57954 which keeps adding the open_basedir folders as dirs to search in does not make sense.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stof so does this PR now. Symfony 6 and 7 also do not check whether the paths are allowed by open_basedir - the check happens by silencing open_basedir errors when using is_dir and is_executable with @. See

foreach ($searchPath as $path) {
// Silencing against https://bugs.php.net/69240
if (@is_dir($path)) {
Expand All @@ -61,11 +66,6 @@ public function find(string $name, ?string $default = null, array $extraDirs = [
}
}
}
} else {
$dirs = array_merge(
explode(\PATH_SEPARATOR, getenv('PATH') ?: getenv('Path')),
$extraDirs
);
}

$suffixes = [''];
Expand Down
40 changes: 39 additions & 1 deletion src/Symfony/Component/Process/Tests/ExecutableFinderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,13 @@ public function testFindWithOpenBaseDir()
$this->markTestSkipped('Cannot test when open_basedir is set');
}

$initialOpenBaseDir = ini_set('open_basedir', \dirname(\PHP_BINARY).\PATH_SEPARATOR.'/');
$openBaseDir = \dirname(\PHP_BINARY).\PATH_SEPARATOR.sys_get_temp_dir().\PATH_SEPARATOR.getcwd();

if ($_SERVER['SYMFONY_PHPUNIT_DIR'] ?? null) {
$openBaseDir .= \PATH_SEPARATOR.$_SERVER['SYMFONY_PHPUNIT_DIR'];
}

$initialOpenBaseDir = ini_set('open_basedir', $openBaseDir);

try {
$finder = new ExecutableFinder();
Expand All @@ -121,6 +127,38 @@ public function testFindWithOpenBaseDir()
}
}

/**
* @runInSeparateProcess
*/
public function testFindWithSubdirectoryOfOpenBaseDir()
{
if (\ini_get('open_basedir')) {
$this->markTestSkipped('Cannot test when open_basedir is set');
}

$paths = explode(\PATH_SEPARATOR, getenv('PATH'));
$phpBinaryPath = \dirname(\PHP_BINARY);

if (!in_array($phpBinaryPath, $paths, true)) {
$paths[] = $phpBinaryPath;
}

$this->setPath(implode(\PATH_SEPARATOR, $paths));

$openBaseDir = \dirname(\dirname(\PHP_BINARY)).\PATH_SEPARATOR.sys_get_temp_dir().\PATH_SEPARATOR.getcwd();

if ($_SERVER['SYMFONY_PHPUNIT_DIR'] ?? null) {
$openBaseDir .= \PATH_SEPARATOR.$_SERVER['SYMFONY_PHPUNIT_DIR'];
}

ini_set('open_basedir', $openBaseDir);

$finder = new ExecutableFinder();
$result = $finder->find($this->getPhpBinaryName());

$this->assertSamePath(\PHP_BINARY, $result);
}

/**
* @runInSeparateProcess
*/
Expand Down