'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 would you say this is?

phprealpath


Solution 1:[1]

I use this for a template file, so it can include "pages" instead of having to clutter a single file with tons of functions/strings/switch-cases/elseifs (take your pick) or create tons of files with the same layout.

It checks the realpath of the directory includes should be in and realpath of the file that is to be included, if the realpath of the file starts with the include directory, it is allowed to 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";
    }
}
?>

Solution 2:[2]

Well, actually it realpath in this case doesn't provide any security. Actually it that case it serves no purpose at all, as include internally will expand the path. Your security here actually depends on preg_match. Note however, that regex you're using won't allow you to use an any page with upper case letter, with underscore or dash.

Anyhow, I don't think that including files based on parameters passed in request is good idea. There is something wrong with your design if you need that.

Solution 3:[3]

realpath doesn't help you at in this instance, as include can resolve the page path to the same file, no matter whether it is realpath'd or the original relative. It 'seems' to be valid, but I wouldn't trust that piece of code personally. I'm sure someone figures a neat way to inject a null-character to the input, wreaking havoc to the string termination.

What you need to do instead, to be safe, is keep a whitelist (or blacklist, if it happens to suit better, but prefer whitelists) of all allowed inputs/pages.

Solution 4:[4]

It seems to be safe....

But not efficient. In MVC you have the controller and view dirs preset and pre known. No point in doing a stat to check if view/controller exists.

Solution 5:[5]

realpath() will actually have some effect in this case as it'll return FALSE if the target file does not exits. But as the other posters already said - this will not add any security to the code.

It's rather a way of error-handling in case the designated files do not exist.

Solution 6:[6]

Better run basename() on $_GET['p']. No directory traversal attacks for sure.

Solution 7:[7]

I cannot comment on the PHP realpath, but if it's just a wrapper for the system's realpath function then one thing to be aware of: on some systems (eg, Solaris), if the process receives a signal while realpath is executing it'll return the empty string ... in which case, you'd have to ensure your code is setup to handle that type of situation (unless the PHP implementation resolves that particular dilemma for you)

Sources

This article follows the attribution requirements of Stack Overflow and is licensed under CC BY-SA 3.0.

Source: Stack Overflow

Solution Source
Solution 1
Solution 2
Solution 3 Henrik Paul
Solution 4 Itay Moav -Malimovka
Solution 5 Stefan Gehrig
Solution 6 Paweł Hajdan
Solution 7 Brian Vandenberg

Related Questions