Proper Includes via $_GET

nthitz · Post by **nthitz** » Fri Aug 12, 2005 5:25 pm

Alright for templating systems that use an index to display multiple pages via $_GET['page'] or what not we've all learned not to do just

Code: Select all

include($_GET['page']);

Naturally we'd want some kind of validation to make sure that $_GET['page'] isn't anything deadly. However I began wondering about cross server includes a while back and a huge problem they could cause. Last night I tried it out, I'll give an example of what I did on my server. (domain.com being a different site)

Code: Select all

include('http://www.domain.com/phpbb/config.php');
echo $dbhost.$dbname.$dbuser.$dbpasswd;

I expected the database info for the site but no dice. Is it because I am including a file from a different server? Then why all the problems with the above template system?

Ambush Commander · Post by **Ambush Commander** » Fri Aug 12, 2005 6:23 pm

Okay, here's the dice. If the server's PHP parser is on, the files are safe, because when you try to include the file, the server wil parse it and you'll simply have a blank page.

The big problem is when another person tricks YOU into including the wrong file.

nthitz · Post by **nthitz** » Sat Aug 13, 2005 12:15 am

Wow.
I'm dumb. Thanks for the tip. And no I'm not a n00b. just a stupid question

Post by **onion2k** » Sat Aug 13, 2005 3:40 am

In general, unless I'm going to have millions of different include files, I use a switch case:

Code: Select all

switch ($_GET['file']) {
    case "index": include("index.inc.php"); break;
    case "file1": include("file1.inc.php"); break;
    case "file2": include("file2.inc.php"); break;
    case "file3": include("file3.inc.php"); break;
    default: include("index.inc.php"); break;
}

That completely eliminates the possibility of anyone tricking my code into including something I don't want it to.

theda · Post by **theda** » Sat Aug 13, 2005 8:11 am

Onion, a much simpler way would be to use an array wouldn't it?

Code: Select all

$arra = array('file1.php','file2.php','file3.php','file4'.php);
if (in_array($_GET['id'],$arra)) {
   include $_GET['id']; 
} else {
   exit;
}

Edit: Deprecate your life -_-; <- For Roja.

Todd_Z · Post by **Todd_Z** » Sat Aug 13, 2005 9:34 am

How i do it is I have a folder of safe include pages, with subfolders within that.

Code: Select all

if ( !isset( $_GET['p'] ) )
  $_GET['p'] = "Main";
else if ( is_file("/absolutepath/Pages/LoggedIn/{$_GET['p']}.php") ) {
  if ( isset($_SESSION['id']) )
    $_GET['p'] = "/LoggedIn/{$_GET['p']}";
  else {
    errorBox( "You must be logged in to view this page." );
    unset( $_GET['p'] );
  }
} else if ( is_file("/absopath/Pages/Landing/{$_GET['p']}.php") )
  $_GET['p'] = "/Landing/{$_GET['p']}";
else if ( !is_file("/absopath/Pages/{$_GET['p']}.php") )
  $_GET['p'] = "Error";
if ( $_GET['p'] ) include "/absopath/Pages/{$_GET['p']}.php";

Looks a little messy, but it works really well, never had any problems. If the $_GET['p'] page isn't in the folders you specify, it will never get included. Hackers got nothin on me.

nielsene · Post by **nielsene** » Sat Aug 13, 2005 10:03 am

Well you aren't stripping out "../"s so its possible that a file could escape the sandbox. It is forced to be a php file so they couldn't show the passwd file. However I would suggest some for of cleaning on the filename to remove ../s

Roja · Post by **Roja** » Sat Aug 13, 2005 10:37 am

Todd_Z wrote:If the $_GET['p'] page isn't in the folders you specify, it will never get included.

Not true.

You can set p to include ../'s, as nielsene said.

Here's the test script:

Code: Select all

<?php
var_dump(is_file("/home/Roja/public_html/test/{$_GET['p']}.php"));
var_dump(is_file("test.php"));
?>

If you do:

http://example.com/~Roja/test/test.php? ... /test/test

and test.php is at /home/Roja/public_html/test, it will display true to both.

Which means you can then include a php file from anywhere in the path - not just the subdirectory you defined..

Todd_Z wrote:Looks a little messy, but it works really well, never had any problems. Hackers got nothin on me.

Thats why statements like "I've never had any problems" have no place in a discussion about security. Just because you haven't had a problem, doesn't mean there isn't one.

And yes, hackers do have something on you - some tricks and knowledge.

Todd_Z · Post by **Todd_Z** » Sat Aug 13, 2005 11:17 am

if they try to include /absolutepath/../../../index.php, it will fail.

Roja · Post by **Roja** » Sat Aug 13, 2005 11:40 am

Todd_Z wrote:if they try to include /absolutepath/../../../index.php, it will fail.

Correct, because you are going above the root - which makes no sense.

But since you use an absolute path, they can go from root down - ie, any php page on your system, NOT just the subdirectories, as you claimed.

Todd_Z · Post by **Todd_Z** » Sat Aug 13, 2005 11:50 am

explain to me how if I am including "/home/blah/public_html/{$_GET['p']}/" how a hacker could view files above the public_html folder? If you tried to include /home/blah/public_html/../../index.php", you would get an error for file not being found.

Roja · Post by **Roja** » Sat Aug 13, 2005 11:53 am

Todd_Z wrote:explain to me how if I am including "/home/blah/public_html/{$_GET['p']}/" how a hacker could view files above the public_html folder?

They can't VIEW them, they can cause them to be included/executed.

Todd_Z wrote: If you tried to include /home/blah/public_html/../../index.php", you would get an error for file not being found.

Not if index.php is in /home, you wouldn't. Thats what my code example does. Try it!

josh · Post by **josh** » Sat Aug 13, 2005 12:21 pm

Having an include folder and doing this:

Code: Select all

include("/direct/path/includes/" . basename($_GET['p']));

would probably be the "easiest" way to do this, I personally never include anything from user input, I opt to store my content in the database.

nthitz · Post by **nthitz** » Sat Aug 13, 2005 5:51 pm

They way I usually do it is to check for a protocal name in $_GET['page'] So if it has http in it, no dice.

nielsene · Post by **nielsene** » Sat Aug 13, 2005 8:36 pm

jshpro2 wrote:Having an include folder and doing this:
Code: Select all
include("/direct/path/includes/" . basename($_GET['p']));
would probably be the "easiest" way to do this, I personally never include anything from user input, I opt to store my content in the database.

Yup, that's what I was trying to get at. Unless you match with a regexp to "cleanse" the data to ensure it has no ../../s or use an explicit basename, there is a chance they can break out.

Its a very small chance, in this cause because his example is forced to be a php file, but attackers could try some common "cruft" like index-old, or index-bak or debug, etc all of which might be present, etc. A lot of people might have a "testing.php" in their webroot for phpinfo.... so they might be able to find something like that, etc