PHP: Is it safe to include a file based on a GET v

Question

How safe is this?

   if (isset($_GET["var"]) && file_exists("path/".$_GET["var"].".php")) { 
        include("path/".$_GET["var"].".php");
    } else {  
        echo 'File Does Not Exist!';   
    }

I'm wondering if $_GET["var"] needs to be "sanitized" opposed to just letting it run against the file_exists function before trying to include it or not. Is this dangerous?

+++UPDATED+++

Thank you all for your responses! Please see updated below...

function mrClean($var) {
$clean_var = (isset($var) && !empty($var)) ? $var : 'index';
$clean_var = preg_replace('/[^-A-Za-z0-9_]/', '', $clean_var);
return $clean_var;
}

$var = mrClean($_GET["var"]);

if (file_exists("path/".$var.".php")) { 
  include("path/".$var.".php");
} else {  
  echo 'File Does Not Exist!';   
}

When I call on mrClean to replace all, but the following:

- A-Z a-z 0-9 _ via preg_replace

...will this now be considered safe? Is there anything that can be added to make this any safer?

I will implement a whitelist as suggested... but anything else?

Thank you!!

-Andrew

Answer 1

You do not want to do this.

let's say i am running index.php in /var/www/test and inside of that is path (so /var/www/test/path exsts)

if i ask this question - file_exists('path/../../../../etc/nginx/sites-available/default.php')

the answer is YES

i.e. a file in /etc/nginx/sites-available/default.php does exist. All I did was pass in $_GET['var'] as "../../../../etc/nginx/sites-available/default"

so now you have asked the webserver to include some random file into your page.

now - we have to note here that depending on permissions, operating system etc. this may not work the same. But in general, you don't want to do this - go the long way and if statement / switch statement stuff into the includes.

You can try to make sure the input is safe by removing characters etc. but in the end, one mistake will cost you your whole file system - not worth it!

Answer 2

Yes, the regex replace within your question update is SAFE. But be aware of that ANY include is dangerous and if you will allow the user to include some unsafe script.