[Process] do not search in $PATH entries not allowed by open_basedir #58008
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stof
reviewed
Aug 14, 2024
fritzmg
reviewed
Aug 14, 2024
fritzmg
suggested changes
Aug 14, 2024
Comment on lines
56
to
+58
| if (\ini_get('open_basedir')) { | ||
| $searchPath = array_merge(explode(\PATH_SEPARATOR, \ini_get('open_basedir')), $extraDirs); | ||
| $dirs = []; | ||
|
|
There was a problem hiding this comment.
With the new logic $extraDirs do not have to be searched here. Imho #57954 is the correct variant.
There was a problem hiding this comment.
#57954 which keeps adding the open_basedir folders as dirs to search in does not make sense.
There was a problem hiding this comment.
@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
|
Closing in favor of #58291 |
nicolas-grekas
added a commit
that referenced
this pull request
Sep 17, 2024
…sedir (BlackbitDevs) This PR was merged into the 5.4 branch. Discussion ---------- [Process] Fix finding executables independently of open_basedir | Q | A | ------------- | --- | Branch? | 5.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Issues | - | License | MIT This backports #47422 to 5.4, which is a bugfix really. Instead of #58008 and #57954 /cc `@xabbuh` `@fritzmg` Commits ------- 4424763 [Process] Fix finding executables independently of open_basedir
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
replaces #57954
The current version of the
ExecutableFinderonly checks the paths in theopen_basedirwhen set. However, this will cause theExecutableFindernot find the executable in question if it is in a subfolder of one of theopen_basedirpaths.For example the environment might be configured as follows:
PATH=/usr/binopen_basedir=/usrIn this case the
ExecutableFinderonly checks the/usrfolder and won't find the binaries in/usr/bin, even though the PHP process would be allowed to access/usr/bin, as theopen_basedirrestriction allows access to subfolders.This PR fixes that by always adding the paths from
PATHto the directories to be checked.Note: this is not an issue in Symfony 6.4+. The
open_basedirlogic does not exist there and thus that problem does not exist there.