-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Description
Symfony version(s) affected: 5.x
Description
The find()
function in Component/Process/ExecutableFinder.php
attempts to find executables outside of the open_basedir
array if the open_basedir
directive is set in php.ini
.
This is an issue as PHP will throw an error if the application tries to access any file outside of the list specified by the directive. It is suppressed in the function for now using the PHP error control operator (@). However, if an application uses an error handler, registered with set_error_handler
, these errors will bubble up. Either as errors or polluting the logs, depending on the handler's implementation.
The original purpose of using the @
operator was to avoid throwing errors due to a bug in PHP where an error would be thrown if the path was in the open_basedir
directive but didn't exists. This seems reasonable as that may indicate a faulty config and could be caught by a developer or end user if a handler is registerd. But as it stands it will produce errors that aren't actually errors (the application shouldn't find any executables or be looking outside the open_basedir
list) confusing application developers and potentially end users.
How to reproduce
create test.php
in the <project-src-path>
directory:
<?php
include 'Symfony/Component/Process/ExecutableFinder.php';
use Symfony\Component\Process\ExecutableFinder;
set_error_handler(function ($errno, $message, $file, $line) {
$msg = $message . ' at ' . $file . '#' . $line;
echo "ERROR: " . $msg . "\n";
});
$executableFinder = new ExecutableFinder();
$executable = $executableFinder->find("cat", null, ["/bin"]);
var_dump($executable);
?>
and run it with:
cd <project-src-path>
php -d open_basedir="$(pwd)" test.php
You should see a log like:
ERROR: is_dir(): open_basedir restriction in effect. File(/bin) is not within the allowed path(s): (/pwd/path) at pwd/path/Symfony/Component/Process/ExecutableFinder.php#56
Possible Solution
revert commit e238c893e975598f66f66349a3b1e2c374cb8c0b where the following:
$searchPath = explode(PATH_SEPARATOR, ini_get('open_basedir'));
was changed to:
$searchPath = array_merge(
explode(PATH_SEPARATOR, ini_get('open_basedir')),
$extraDirs
);
There is, unless I've missunderstood something, no reason to look in $extraDirs
if open_basedir
is set.
I'd be willing to have a go at a PR but I'm no PHP developer so I'm not 100% that I'll easily be able to setup the environment and handle the testing framework. But perhaps I should give it a go?