Security issue

deep

Hello!

While i was building my new site i came to think about something... you see when i load information into my index.php i simply use this code:

if (isset($_GET['id'])) {
$file = $_GET['id'] . ".php";
include ($file);
}

So if i got about.php and faq.php in the same folder as index.php i simply write www.myaddress.com/index.php?id=about

That way i just insert the information from the about page to my main index and dont need to build the whole about page to look like my index page.

BUT.. what if a hacker then typed something like
www.myaddress.com/index.php?id=http://www.hacheraddress.com/page and that way reads the variables and information beeing transfered on my page?? is that possible or is my method safe?

Thanks!

TruckStuff

Originally posted by deep
is that possible or is my method safe?

Yes, its entirely possible. Including files through GET variables is a remarkably bad idea. This is the exact reason that REGISTER_GLOBALS is now off by default in PHP. By adding this code, you have effectively negated the increased security by disabling REGISTER_GLOBALS.

deep

So how do you suggest that i do instead?

TruckStuff

RTFM. 😃

http://www.php.net/variables.predefined

Weedpacket

TruckStuff's comment is basically irrelevant. You're already accessing your form variables in the manner recommended by PHP (with register_globals turned off, etc.), and there's not really any other way (sensible) to get at them.

Your problem is what you're doing with the form variables. People can stick any path- and filename they like into your form and you'll happily try to serve it to them.

NEVER TRUST USER INPUT!! Don't assume everyone out there is a nice person. Don't even assume they know what they're doing. NEVER use form variables without checking that their contents are (a) legible, and (b) legitimate.

Don't just try and guess what people might try to do and then guard against it: whether through malicious intent or mental incapacity someone will try something you haven't thought of. Instead of disallowing bad input, only allow good input - and throw away anything that doesn't meet your criteria.

In your case, you should keep some sort of list of pages people are allowed to request, and only honour requests for pages that are on that list.

deep

Thanks! That helps alot! 🙂

Sco_________

You could also secure it a bit by doing a few things..

<?
$page=$_GET['page'];

If(isset($page) && !empty($page)){

If(!eregi("^[A-Z0-9]+$", $page)) {

Exit("Invalid Request");

} Else {

include("content_".$page.".php");

}

}
?>

For 1 this only includes files like content$page.php so it's not really possible to use contenthttp://.php etc.. and then the eregi only allows numbers and letters.

superwormy

Often what I end up doing in a situation like yours is using a bunch of if/elses or a switch{} statement for all valid pages.

switch ($_GET['page']
{
case "about":
include ("about_page.html.php");
break;
case "contact":
include ("contact_page.html.php");
break;
default:
print ("Invalid page requested!");
exit;
}

Weedpacket

If your page names are based on the options, then you could use in_array() to see if the requested option is valid (i.e., in the array of legitimate options), and then use the include($option."_page.php") method.

DominicIGO

I'm a newb so what I'll say might be completely stupid but... shouldn't this do the job?

if (isset($_GET['id'])) { 
$file = "http://www.youraddress.com/" .  $_GET['id'] . ".php"; 
include ($file); 
}

That way only files from your own site can be included?

superwormy

That won't work at all, you can't include() over HTTP and expect to get anything except the pages generated HTML...