#1
  1. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2009
    Posts
    157
    Rep Power
    89

    File inclusion, security


    Hi,

    Can I please get some comment as to how secure/unsecure the following code is?

    PHP Code:
    $dir "content";
    $folder scandir($dir);
    foreach (
    $folder as $file)
    {
        if (
    substr($file, -4) == ".php"$files[] = $file;
    }

    $page $_GET['page'];
    if (
    in_array("$page.php"$files)) include "content/$page.php"
  2. #2
  3. No Profile Picture
    Contributing User
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2003
    Posts
    3,600
    Rep Power
    595
    What exactly is your concern. There is not enough information here to tell what you are doing.
    There are 10 kinds of people in the world. Those that understand binary and those that don't.
  4. #3
  5. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2009
    Posts
    157
    Rep Power
    89
    Originally Posted by gw1500se
    What exactly is your concern. There is not enough information here to tell what you are doing.
    If the code was as follows

    PHP Code:
    include $_GET['page']; 
    then url/?page= could be manipulated to include anything on the file system.

    I am trying to allow inclusion of only php files stored in a specific directory based on the $_GET['page'] variable without it being exploitable.
  6. #4
  7. No Profile Picture
    Contributing User
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2003
    Posts
    3,600
    Rep Power
    595
    I'm still not sure I understand. Are you saying you have pages in DocRoot that you do not want to be accessible? That in and of itself is a bad practice. You should have those pages outside of DocRoot. Then validate the $_GET URL and allow or disallow it based on whatever your criteria is.
    There are 10 kinds of people in the world. Those that understand binary and those that don't.
  8. #5
  9. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Apr 2009
    Posts
    157
    Rep Power
    89
    The criteria is

    Files are of extension .php
    Files exist in nominated directory

    if it passes those two criteria, include file otherwise ignore

    The code I posted in the original message is the validation.

    I wanted to verify if it was exploitable to allow inclusion of files which do not meet that criteria.
  10. #6
  11. No Profile Picture
    Contributing User
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2003
    Posts
    3,600
    Rep Power
    595
    The key is whether or not the files are in DocRoot. If not, then security is as strong as your validation. If they are, then exploitation is certainly possible but even then, the Apache config file for that directory can nail it down pretty tight. Either way, if your validation is good then your security is good.
    There are 10 kinds of people in the world. Those that understand binary and those that don't.
  12. #7
  13. No Profile Picture
    Lost in code
    Devshed Supreme Being (6500+ posts)

    Join Date
    Dec 2004
    Posts
    8,316
    Rep Power
    7171
    On initial glance it looks fine, but it's not particularly efficient to scan the entire directory on every page load. You can secure the value without needing to do that just by strictly limiting the characters it may contain. Normally for a simple script I route using something like this:
    PHP Code:
    $dir "content";
    $page = isset($_GET['page']) ? $_GET['page'] : 'default-page';

    if(!
    preg_match('#^[a-z0-9_-]+$#'$page) || !file_exists($dir '/' $page '.php'))
    {
      
    $page '404';
    }

    require(
    $dir '/' $page '.php'); 
    What gw1500se mentioned about direct access to the pages is worth considering too; obviously if $dir is externally accessible then someone could type the direct path to $page instead of going through your front controller file.

    Comments on this post

    • TASB agrees : cheers, that is a lot cleaner.
    PHP FAQ

    Originally Posted by Spad
    Ah USB, the only rectangular connector where you have to make 3 attempts before you get it the right way around

IMN logo majestic logo threadwatch logo seochat tools logo