PHP code to show a PHP page

Question

I'm trying to avoid manually updating a whitelist of pages, so I wrote the following that I might use, but I'd like my fellow, more experienced, developers to let me know if this is safe. By safe, I mean that it won't allow the inclusion or execution of arbitrary code. Is it safe from directory traversal and other similar vectors?

<?php
$current_page = 'default_page';
//Get list of acceptable pagenames:
$found_pages = [];
if ($handle = opendir('pages')) {
    while (false !== ($entry = readdir($handle))) {
        if ($entry != "." && $entry != "..") {
            $found_pages[]= $entry;
        }
    }
    closedir($handle);
}

if(isset($_GET)){
  $requested_page=reset($_GET); //just need value of first GET element, regardless of name.
  if(in_array($requested_page.'.php',$found_pages)){
    $current_page=$requested_page;
  }
}

include('pages/' . $current_page . '.php');

Answer 1

Yes, I believe that will accomplish what you are trying to do. However, I think it can also be streamlined pretty substantially. Some general pointers:

1. Looping through the directory to get all files on all page requests is going to be pretty inefficient in the long-run. File operations are relatively expensive, because they require disk-seeks. Obviously this is less critical for a low-traffic system with a small number of files.
2. You are returning the default page if an invalid page is requested. I think it would be better to either show an access-denied error (as such a request might signify someone trying to circumvent restrictions in your system) or at least a 404 (which would be more friendly towards legitimate users). Either way, you should stop executing when an invalid page is found.
3. You could fix the first issues if you have a database in your system. Instead of directly checking the file path, you could record the list of allowed pages in your database, check against the DB, and then load the page in much the same way you currently are. The advantage of such a solution is that you could separate your public paths from your filenames: the database could record the "url" of the file which you lookup against, and then fetch the actual filename out of the database. Of course if you do that, you pretty much just made a basic routing system.
4. Is there anything that prevents users from going directly to the pages? From the looks of it your pages are just a sub-directory of your public system. That may suggest room for possible security vulnerabilities.

As an alternate way of accomplishing exactly what you are doing without reading the full directory, you can use realpath. It returns the normalized absolute URL to a file, or nothing if the file doesn't exist. You can then check and see if what is returned by realpath actually lives in your application directory. That will make for fewer disk seeks and will allow your performance to remain (relatively) constant even if the number of files grows. Here is a quick example:

if(isset($_GET)){
    $pages_dir='/path/to/pages/folder/pages/';
    $requested=reset($_GET);
    $path=realpath($pages_dir . $requested);
    if(!$path || substr($path, 0, strlen($pages_dir)) != $pages_dir){
        show_404();
        exit;
    }
}

include($path);

To reiterate, you build the path assuming the file lives in the destination directory. realpath will then resolve the path, accounting for directory traversals. I.e. if $_GET contained ../../../../../../../etc/passwd then you would pass this to realpath: /path/to/pages/dir/pages/../../../../../../../etc.passwd and realpath would return simply /etc/passwd. You then check what realpath returned to see if your results are still in the proper directory. If it is, then you're good.