<?php
$page = $_GET['page'];
if(file_exists("$page.php")) include("$page.php");
else header( 'Location: http://www.example.com/index.php?page=main' );
?>
Refactorings
No refactoring yet !
techietim
October 4, 2007, October 04, 2007 14:36, permalink
Check this out:
http://refactormycode.com/codes/42-content-page-include
ody.myopenid.com/
October 4, 2007, October 04, 2007 23:54, permalink
The above shows an example of how to check that 'page' is set in GET and how we can use a simple array to verify that the page is allowed to be accessed. You should check out the Suhosin-Patch which provides stricter access to files by allowing you to set which directories PHP can access.
<?php
$pages = array('default', 'page1', 'page2');
if(isset($_GET['page']))
{
$page = $_GET['page'];
}
else
{
$page = 'default';
}
if(in_array($page, $pages))
{
if(file_exists("$page.php"))
{
include("$page.php");
}
else
{
header( 'Location: http://www.example.com/index.php?page=main' );
}
}
JWvdV
October 5, 2007, October 05, 2007 07:42, permalink
Here you've 2 ideas.
@ody: Why are you using double-quotes? It's really bad for debugging. Besides that, your code can cen be less.
<?php
/* First we create an array of permitted page-files */
$accessiblepages = Array('main', 'page1', 'page2');
if(isset($_GET['page']) && in_array($_GET['page'], $accessiblepages) && file_exists($_GET['page'].'.php')) include_once($_GET['page'].'.php');
else include_once('main.php');
?>
<?php
/* An other idea is to put al pages in een directory with all accessible files */
if(isset($_GET['page']) && file_exists('./allpagesyouareallowedtosee/'.$_GET['page'].'.php')) include_once('./allpagesyouareallowedtosee/'.$_GET['page'].'.php');
else include_once('./allpagesyouareallowedtosee/main.php');
?>
H3lpd3sk
October 6, 2007, October 06, 2007 05:12, permalink
Maybe you should take a look at this thread: http://refactormycode.com/codes/42-content-page-include
FiSh
October 7, 2007, October 07, 2007 19:30, permalink
All of the above are very insecure implementations allowing inclusion of files like /etc/passwd with a null byte in the url. This will take out relative paths and null bytes.
<?php
$bad = array("..", "\0", "/", "\\");
$u = str_replace($bad, "", $_GET['p']) . ".php";
if(file_exists($u)) include($u);
else die("Invalid page.");
?>
Mike Weller
October 11, 2007, October 11, 2007 14:57, permalink
@ FiSh, blacklisting with regexps is never the right solution. There will always be some special characters or something you have missed.
Instead, a white list of allowable files should be used, as the other solutions have offered.
Felix
October 20, 2007, October 20, 2007 12:33, permalink
With alternative if syntax
<?php
$allowedPages = array('imprint','news','main'); //add more
include_once (isSet($_GET['page']) && in_array($_GET['page'],$allowedPages) && file_exists($_GET['page'].'.php'))?$_GET['page'].'.php':'error.php';
?>
Diggler
October 24, 2007, October 24, 2007 21:01, permalink
Hello, this is my version for a secure file include:
it is very important that you don't use include($file.".php");
This is very vulnerable because of remote file inclusion.
You have to set a local path like "./" That's all you need!
But first of all you should check the input.
<?php
# Function removeInjection(GPC)
function removeInjection(&$aData)
{
foreach($aData as $k => $v)
{
if(is_array($aData[$k]))
{
removeInjection($aData[$k]);
} else {
$aData[$k] = htmlentities(strip_tags($v), ENT_QUOTES);
}
}
}
//Remove any code injection
removeInjection($_GET);
$file = $_GET['site'];
if(file_exists("./".$file.".php") && $file != "index")
{
include("./".$file.".php");
} else {
include("./default.php");
}
?>
is there a better and safer/ more secure way of doing this?