2
\$\begingroup\$

I found a lot of working codes to include page in page on Internet but I could not find a safe code. So I decided to create one myself. The pages will be only stored in folder /pages/ and whitelist seems to be a good option.

Is the follow code safe?

<?php
$unsafe = $_GET['pagename'];
$page = preg_replace('/[^A-Za-z0-9\-]/', '', $unsafe);
if (empty($page)){
include('pages/default.php');
}
$pages = array('default', 'pageone', 'pagetwo', 'another', 'last');
if (isset($pages[$page])) {
    include('pages/' . $page . '.php');
} else {
include('pages/error-404.php');
}
?>
\$\endgroup\$
  • \$\begingroup\$ use file_exists() to check the file existence. \$\endgroup\$ – bekt Mar 9 '15 at 4:16
  • \$\begingroup\$ using file_Exists isn't really a safe option. You would need a lot of extra sanitation before passing something into file_exists. Just think of ../../et/password \$\endgroup\$ – Pinoniq Mar 14 '15 at 14:16
1
\$\begingroup\$

You are checking if a certain key exists in an array: isset($pages[$page]) but you define the $pages array without keys, so they default to numeric ones. A much better approach is to skip the preg_replace thing (it is useless anyway) and create a router array:

$router = array( 'url' => 'pages/foobar.php', 'default' => 'pages/default.php' );

This array maps urls to your internal pages. Then you check or the requested page exists:

if (isset($router[$_GET['page']]) {
    include $router[$_GET['page'];
} else {
    include $router['default'];
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.