Skip to content
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

_ss_environment checks break under open_basedir #1866

Closed
simonwelsh opened this issue May 10, 2013 · 3 comments
Closed

_ss_environment checks break under open_basedir #1866

simonwelsh opened this issue May 10, 2013 · 3 comments

Comments

@simonwelsh
Copy link
Contributor

With be78098 (by @dhensby, merged by @chillu), the entire directory path is searched for _ss_environment.php. If there's an open_basedir() in effect, it's rather unlikely to allow this to happen.

@dhensby
Copy link
Contributor

dhensby commented May 11, 2013

Hi @simonwelsh

I thought about this with the patch, but that'd be the case with the old way too. If PHP wasn't allowed outside the webroot, then the same error would have happened.

It is a warning, not an error, so we can't do try catch

We could add a @is_readable() in there as a way of breaking out...

@dhensby
Copy link
Contributor

dhensby commented May 22, 2013

Ok, so I've submitted a patch for this issue.

There were a couple things wrong with the logic that I've now fixed. The environment file spider logic was only using the realpath, which is ok, but not fool proof as it could be in the path that isn't the 'real' one. So this logic now spiders 2 dirs (if they are different).

It also checks the dir is readable whilst suppressing the warnings that would come out due to open_basedir settings AND it breaks out of the loop if the dir isn't readable (making an assumption that parent dirs won't be readable if the current one isn't).

@chillu some peer review is welcome :)

@dhensby
Copy link
Contributor

dhensby commented May 22, 2013

@chillu Ok, I re-jigged this quite a bit (again) so that it's using dirname, not arrays or exploding on DIRECTORY_SEPERATOR. Much more elegant.

@chillu chillu closed this as completed in ac0e324 May 22, 2013
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

No branches or pull requests

2 participants