Skip to content

Fix handling of open_basedir that contains cwd #7015

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.

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

Conversation

nikic
Copy link
Member

@nikic nikic commented May 19, 2021

While resolving the path, the last step will reduce it down to ""
(an empty string) and realpath() will resolve this to getcwd().
If open_basedir contains the CWD, then that means open_basedir
will be bypassed for paths that don't have any components that
exist (if one of the components exists, then we abort the realpath
loop at that point).

While resolving the path, the last step will reduce it down to ""
(an empty string) and realpath() will resolve this to getcwd().
If open_basedir contains the CWD, then that means open_basedir
will be bypassed for paths that don't have any components that
exist (if one of the components exists, then we abort the realpath
loop at that point).
@nikic nikic requested a review from cmb69 May 19, 2021 14:20
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Good catch! Although it is just about the missing open_basedir warning from what I can tell, we should consider targeting PHP 7.4 or 8.0.

@nikic
Copy link
Member Author

nikic commented May 19, 2021

Good catch! Although it is just about the missing open_basedir warning from what I can tell, we should consider targeting PHP 7.4 or 8.0.

I think the effect here is slightly larger than only the warning: It should also be possible to create files/directories outside open_basedir. Of course, that requires that the PHP process itself has permissions to write /, e.g. running as root. Which one probably should only be doing when running in Docker.

@cmb69
Copy link
Member

cmb69 commented May 19, 2021

Right! I was assuming not running as root, and reasonable permissions. Otherwise, that would be a rather serious security issue.

FWIW, this issue apparently doesn't affect Windows. Still good to have the ckeck there as well.

@nikic nikic closed this in ee7a8ac May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants