I explained myself incorrectly.
Basically, if I remove the first few elements of the test, which all evaluate to false, we find ourselves with:
if (strpos($file, $real_source) === false)
fatal_error(..)
Which evaluates to true for anything that is NOT in the Sources folder... (Even a root file will thus return an error, as opposed to what I said last night. It was very, very late for me...)
Hence the bug.
Also, it evaluates for $cachedir later in the line... Which doesn't make much sense to me..?! :edit: Oh yes it does, it ensures we shouldn't be trying to read a cache file...
Oh, and these extra brackets 'joining' the $boarddir and $sourcedir tests are weird and unneeded :P It really feels like a && test that became a || test and wasn't reprocessed entirely. Which is the case, but you know how much of a nazi I am for these things... :^^;:
Posted: January 13th, 2013, 09:51 AM
So, basically, I've rewritten it to this...
if (strrchr($basename, '.') != '.php' || $basename == 'settings.php' || $basename == 'settings_bak.php' || strpos($file, $cachedir) !== false || (strpos($file, realpath($boarddir)) === false && strpos($file, realpath($sourcedir)) === false) || !is_readable($file))
I'm putting all of the 'light' work ahead, then the longer operations (realpath and is_readable) at the end, only one realpath is done, except if $boarddir isn't found in the path, in which case another realpath is done against $sourcedir (in the unlikely case $sourcedir isn't part of the $boarddir path...), and that should be it.
At the very least, that line works on my setup. Is it okay if I commit this?
PS: obviously, the $realxxx initialization lines are removed. This is to avoid doing a realpath(), as fast (or slow) as they might be, when the tests aren't even used. Well, it's an obscure admin feature so the only thing that wasted time here is *me* actually, but whatever... :lol:
Posted: January 13th, 2013, 12:03 PM
Changed line to this...
if (strrchr($basename, '.') != '.php' || $basename == 'settings.php' || $basename == 'settings_bak.php' || (strpos($file, realpath($boarddir)) === false && strpos($file, realpath($sourcedir)) === false) || strpos($file, realpath($cachedir)) !== false || !is_readable($file))
I'm pretty sure people would want to protect against root_forum/Sources/../cache/randomname.php ;)
Although I don't really see how they could reach a critical cache file with a random filename, but whatever...!