User input include

Question

This is pretty darn safe right? Something I've missed?

$page = sprintf("%s/%s.php", "pages", $_GET['page']);
if (file_exists($page)) {
 include $page;
}
else {
 echo "The page '$page' does not exist =(";
}

(yes you can use it)

Answer 1

It's extremely unsafe since a user can arbitrarily load whatever page they feel like.

Answer 2

if you are sure there's no / or .. in $_GET['page'], and that the visitor is allowed to view any php file in the pages dir, i think it's okay.

Answer 3

The "better" way to do this is to have an array of the allowed pages, then do something like this:

$page = $_GET['page'] . '.php';
if(in_array($page, $all_pages)) {
    include('pages/' . $page);
}

You could easily get a list of all allowed pages by doing something like this:

$all_pages = glob('pages/*.php');

Documentation: in_array, glob

Answer 4

Use basename() to make sure there's no path information being supplied :

$page = sprintf("%s/%s.php", "pages", basename($_GET['page']));
if (file_exists($page)) {
        include $page;
}
else {
        echo "The page '$page' does not exist =(";
}