Re: file_exists

Instruct ICC wrote:

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"

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

I meant if $page evaluates to "/home/stut/phpstuff/inc/../../../../../../../../../../../../home/evil-user-home-dir/evil-payload.php" which it does not. However I don't think your if (substr($page, 0, strlen($expecteddir)) != $expecteddir) ever evaluates to TRUE. So you'll never get Access denied. So how you set $page saved your ass. Good job.

You clearly don't know what the realpath function does. Look it up. -Stut -- http://stut.net/