Fixed issue with open basedir on windows #1828

Closed
wants to merge 1 commit into
from

Projects

None yet

3 participants

@dirkjanm

When Joomla tries to create a directory or file, it checks whether the path is in the "open basedir" of PHP when this is active.
The path checking is done in a case sensitive way. However on Windows, paths are NOT case sensitive.

Thus lets say my open basedir is:

E:\sample\DIR\

and Joomla thinks my directory is

E:\sample\dir\

The check will fail (even though the directory is perfectly writeable).

Not to mention that this check seems quite overkill since any subdirectory of the website is always within the open basedir path. Also this can be handled on file creation instead of checking this manually.

This error prevents users from saving their settings so it is best fixed as soon as possible.
Attached code makes the checking case insensitive

@dongilbert
Contributor

Some advanced configs might move core files outside of the open_basedir, so this check does need to be made. Also, it is a security feature, so you'll need some good unit tests to back up the change.

With that said, I'm not a fan of the filesystem package that this is a member of, so any attempts to improve it are welcome. :)

Finally, I'm on a *nix box, so I can't verify this as an issue. Maybe @mbabker can help here?

@dirkjanm

I disagree that this is a security feature. It is a feature that gives you a nice error message.
Files outside the open_basedir cannot be written to, that is the security.
Detecting whether you are trying to write to a file outside the open_basedir does not make it anymore secure (unless you are somehow assuming that the open_basedir security does not work on php).
However since this check gives false positives at the moment it is more of an annoyance than any kind of feature.

@dongilbert
Contributor

Valid points. You're correct that it is a nice error message instead of a fatal, which I would hope you want to avoid. :)

So, yes it is an annoyance. Are you trying to fix it for the CMS? Or for a custom app you've written using the platform? (So I can best direct you on how to proceed.)

@dirkjanm

I think open basedir restrictions raises warnings instead of fatal errors (in PHP that is).
I'm trying to fix it for the CMS. I've had it a few times that one of my customers had this issue with hosting on Windows + IIS.
Fixing it in the core manually there will bring the error back when they update.

@dongilbert
Contributor

Ok. The absolute fastest way for you to see the fix is to create an override. Even if we merged this today (which can happen, if a maintainer with a windows machine can verify the fix), it wouldn't make it into the CMS for a while. However, there are some issues on CMS 2.5 that would require a little extra effort to be able to override it. What CMS version are you using?

Sent from my iPhone

On Feb 17, 2013, at 10:31 AM, Dirk-jan notifications@github.com wrote:

I think open basedir restrictions raises warnings instead of fatal errors (in PHP that is).
I'm trying to fix it for the CMS. I've had it a few times that one of my customers had this issue with hosting on Windows + IIS.
Fixing it in the core manually there will bring the error back when they upgrade.


Reply to this email directly or view it on GitHub.

@dirkjanm

Version 2.5.9

@dongilbert
Contributor

Ok. That's a bit of a challenge to override because that specific class is not supported by the auto loader. Classes are jimport()'d instead. However, I've created a process to allow for overriding these items. I'm away from my laptop right now, but I'll put an explanation together for you when I get a chance later today.

Sent from my iPhone

On Feb 17, 2013, at 10:41 AM, Dirk-jan notifications@github.com wrote:

Version 2.5.9


Reply to this email directly or view it on GitHub.

@dongilbert
Contributor

Here's a basic explanation in screenshot form. http://d.pr/i/dcx

Sent from my iPhone

On Feb 17, 2013, at 10:41 AM, Dirk-jan notifications@github.com wrote:

Version 2.5.9


Reply to this email directly or view it on GitHub.

@dongilbert
Contributor

By the way, the benefit of going through this exercise instead of editing
the file in question directly is that when upgrading you only need to worry
about keeping your customized libraries/loader.php up to date rather than
every jimport()'d file that you make edits to.

On Sunday, February 17, 2013, Don wrote:

Here's a basic explanation in screenshot form. http://d.pr/i/dcx

Sent from my iPhone

On Feb 17, 2013, at 10:41 AM, Dirk-jan <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>
wrote:

Version 2.5.9


Reply to this email directly or view it on GitHubhttps://github.com/joomla/joomla-platform/pull/1828#issuecomment-13688671.

@dirkjanm

Thanks, I appreciate your help here. The screenshot doesn't bring me much further, if you have time to explain later I'll wait for that.
Otherwise, this is just a problem that pops up every few months so its not much of a hurry, I just know it will occur again at some point, so if in a few months this could be in the official package it is fine for me.

@eddieajau
Contributor

I'm going to close this one only because the CMS is going to take over maintenance of the Platform. Please resubmit against the CMS repo. Thanks.

@eddieajau eddieajau closed this Mar 16, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment