including pages

[codrgii]

is this secure?

 

if (!empty($_GET['act']) && (file_exists('./page/'.ggg($_GET['act']).'.php'))) {
include('./pages/'.($_GET["act"]).'.php');

 

if not then does anyone have any good examples to make it securer?

[premiso]

Looks alright to me. I would go as far to say that I would setup an array "white" list instead though:

 

$page = isset($_GET['act'])?strtolower($_GET['act']):'index'; // default to index if no data
$validPages = array("index", "view", "user", "contact"); // example white list

if (in_array($page, $validPages)) {
    include('./pages/'.$page.'.php');
}

 

But your way is secure, given that you verify the file_exists. Just another way of doing it

[PFMaBiSmAd]

I would recommend validating exactly what is in the $_GET variable using the method that premiso just posted.

 

Consider this sequence of events - you are including content into a general purpose index.php page using this method and you also have an admin section on your site with its own index.php page that is setup with its' own include files for the admin functions on your site. Someone can use directory transversal by suppling the correct ../../../path_to_your_admin_include_page/ and cause any of the the admin include files to be included on the current page. The file_exists() logic WILL be TRUE but they have just accessed your admin functions and can do anything an admin has permission to do.