Page 1 of 4

Proper Includes via $_GET

Posted: Fri Aug 12, 2005 5:25 pm
by nthitz
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?

Posted: Fri Aug 12, 2005 6:23 pm
by Ambush Commander
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.

Posted: Sat Aug 13, 2005 12:15 am
by nthitz
Wow.
I'm dumb. Thanks for the tip. And no I'm not a n00b. just a stupid question :D

Posted: Sat Aug 13, 2005 3:40 am
by onion2k
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.

Posted: Sat Aug 13, 2005 8:11 am
by theda
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. :P

Posted: Sat Aug 13, 2005 9:34 am
by Todd_Z
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.

Posted: Sat Aug 13, 2005 10:03 am
by nielsene
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

Posted: Sat Aug 13, 2005 10:37 am
by Roja
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. :)

Posted: Sat Aug 13, 2005 11:17 am
by Todd_Z
if they try to include /absolutepath/../../../index.php, it will fail.

Posted: Sat Aug 13, 2005 11:40 am
by Roja
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.

Posted: Sat Aug 13, 2005 11:50 am
by Todd_Z
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.

Posted: Sat Aug 13, 2005 11:53 am
by Roja
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!

Posted: Sat Aug 13, 2005 12:21 pm
by josh
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.

Posted: Sat Aug 13, 2005 5:51 pm
by nthitz
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.

Posted: Sat Aug 13, 2005 8:36 pm
by nielsene
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