Clever Geek Handbook

Is RealPath safe?

<?php
if (preg_match('/^[a-z0-9]+$/', $_GET['p'])) {
$page = realpath('pages/'.$_GET['p'].'.php');
$tpl = realpath('templates/'.$_GET['p'].'.html');
if ($page && $tpl) {
    include $page;
    include $tpl;
} else {
    include('error.php');
}
}
?>

How safe is it to say?

0


source to share


7 replies


I use this for a template file, so it can include "pages" instead of cluttering one file with many functions / lines / switches / elseif (choose) or create many files using the same location.

It checks if the realpath should include the directory, and if it should be included in the real path of the file, which should be included, if the real path of the file starts from the include directory, it can be included.



<?
#If you're worried about funky characters messing with stuff, use this
#preg_replace("/[^A-Za-z0-9_\-]+/","",$str);

if (isset($_REQUEST['page'])) {
    $path=realpath("../inc/page").DIRECTORY_SEPARATOR;
    $full_page=realpath($path.$_REQUEST['page'].".php");
    if (file_exists($full_page)&&(strpos($full_page,$path)===0)) {
        include($full_page);
    } else {
        echo "FAILED";
    }
}
?>
+2


source


Well, realpath

it doesn't actually provide any security in this case. In fact, this is the case when it is impractical, since it will include

internally expand the path. Your safety here actually depends on preg_match

. Note, however, that the regex you are using will not allow you to use any page with an uppercase letter with an underscore or dash.



Anyway, I don't think it is a good idea to include files based on the parameters passed in the request. There is something wrong with your design if you need it.

+1


source


realpath will not help you with this instance, as it include

can resolve the page path to the same file, whether it is realpath'd or the original sibling. It "seems" really, but I wouldn't trust this snippet personally. I'm sure someone knows how to neatly enter a null character into the input, breaking the chaos before the line ends.

To be on the safe side, what you need to do is keep a whitelist (or blacklist if better suited but prefers whitelisting) of all allowed entries / pages.

0


source


It seems to be safe ...

But ineffective. In MVC, you have a controller and settings view and preview. There is no point in doing a stat to check if the view / controller exists.

0


source


realpath()

will indeed have some effect in this case, as it will return FALSE

if the target file doesn't exit. But as other posters have said, this won't add any security to the code.

It is rather a way to handle errors if the assigned files do not exist.

0


source


Better to run basename () on $ _GET ['p']. There is definitely no directory traversal workaround.

0


source


I can't comment on the actual PHP trajectory, but if it's just a wrapper for the system's realpath function, then one thing to be aware of: on some systems (Solaris for example), if a process gets signaled when realpath executes it 'll return an empty string ... in this case, you will need to make sure your code is configured to handle this type of situation (unless the PHP implementation resolves this dilemma for you)

0


source






More articles:


All Articles