Proper Includes via $_GET

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

nthitz
Forum Newbie
Posts: 5
Joined: Fri Aug 12, 2005 5:03 pm

Proper Includes via $_GET

Post 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?
User avatar
Ambush Commander
DevNet Master
Posts: 3698
Joined: Mon Oct 25, 2004 9:29 pm
Location: New Jersey, US

Post 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.
nthitz
Forum Newbie
Posts: 5
Joined: Fri Aug 12, 2005 5:03 pm

Post by nthitz »

Wow.
I'm dumb. Thanks for the tip. And no I'm not a n00b. just a stupid question :D
User avatar
onion2k
Jedi Mod
Posts: 5263
Joined: Tue Dec 21, 2004 5:03 pm
Location: usrlab.com

Post 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.
theda
Forum Contributor
Posts: 332
Joined: Sat Feb 19, 2005 8:35 am
Location: USA

Post 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
User avatar
Todd_Z
Forum Regular
Posts: 708
Joined: Thu Nov 25, 2004 9:53 pm
Location: U Michigan

Post 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.
Last edited by Todd_Z on Sat Aug 13, 2005 11:18 am, edited 1 time in total.
User avatar
nielsene
DevNet Resident
Posts: 1834
Joined: Fri Aug 16, 2002 8:57 am
Location: Watertown, MA

Post 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
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Post 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. :)
User avatar
Todd_Z
Forum Regular
Posts: 708
Joined: Thu Nov 25, 2004 9:53 pm
Location: U Michigan

Post by Todd_Z »

if they try to include /absolutepath/../../../index.php, it will fail.
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Post 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.
User avatar
Todd_Z
Forum Regular
Posts: 708
Joined: Thu Nov 25, 2004 9:53 pm
Location: U Michigan

Post 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.
Roja
Tutorials Group
Posts: 2692
Joined: Sun Jan 04, 2004 10:30 pm

Post 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!
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Post 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.
nthitz
Forum Newbie
Posts: 5
Joined: Fri Aug 12, 2005 5:03 pm

Post 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.
User avatar
nielsene
DevNet Resident
Posts: 1834
Joined: Fri Aug 16, 2002 8:57 am
Location: Watertown, MA

Post 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
Post Reply