5

I want to make sure a file path set via query string does not go outside of the desired subdirectory. Right now, I am checking that:

  1. The path does not start with "/", to prevent the user from giving an absolute path.
  2. The path does not contain "..", to prevent the user from giving a path that is outside of the desired subdirectory.
  3. The path does not contain ":", to prevent the use of a url (i.e. "http://", "ftp://", etc.). Should I ever run this script on a Windows server (not likely), this will also prevent absolute paths beginning with a drive specifier (i.e. "C:\"). Note: I'm aware that a colon is a valid character in a Unix filenames, but I will never be using it in a filename.
  4. The path does not start with "\". Just in case I change my mind about running on a Windows server, this prevents Windows network paths from being specified (i.e. "\\someserver\someshare"). Again, I'm aware that a backslash is a valid Unix filename character, but I also won't be using it in any filenames.

Are these checks sufficient?

Background

I have a PHP script that takes (via query string) the path to a sample source file to be shown to a user. So I might give them a link like "view_sample.php?path=accounting_app/report_view.php" or "view_sample.php?path=ajax_demo/get_info.js".

The script looks basically like this:

$path = $_GET['path'];
if(path_is_valid($path) && is_file("sample/$path"))
{
  header('Content-Type: text/plain');
  readfile("sample/$path");
}

My concern is that a malicious user would see the url and try to do something like "view_sample.php?path=../../database/connection_info.php" and gain access to a file which is not in the "sample" directory.

Are the four checks I defined above (which would be implemented in the path_is_valid() function) sufficient to lock out a malicious user? (Also, I think checks 1, 3, and 4 are basically irrelevant since I am prepending a relative path, but if I didn't do this would the checks be sufficient?)

Kip
  • 107,154
  • 87
  • 232
  • 265

3 Answers3

10

Call

$path = realpath("sample/$path");

Then check that the resulting path starts with the directory you're expecting.

sth
  • 222,467
  • 53
  • 283
  • 367
  • NO, realpath does no such thing. realpath() expands all symbolic links and resolves references to '/./', '/../' and extra '/' characters in the input path . and return the canonicalized absolute pathname. http://ca.php.net/manual/en/function.realpath.php – Samuel Jan 19 '09 at 05:05
  • Isn't that what's needed (minus the symbolic links resolution maybe)? If you have the path in a canonicalized form you can easily find which directory it really references (check if it starts with '/var/www/data/' or wherever your data is). – sth Jan 19 '09 at 05:12
  • actually this sounds like it will do what i'm wanting.. if somehow the user specifies a relative path that's still in the sample directory, i really don't care (i.e. "../sample/.././sample/./.././sample/thefile.php"). – Kip Jan 19 '09 at 05:13
  • Don't count on `realpath` to protect you from a [directory traversal attack](https://en.wikipedia.org/wiki/Directory_traversal_attack). @Samuel is right. – Flimm Feb 01 '16 at 15:33
  • This only works if the directory `sample/$path` exists, otherwise realpath returns false! – Adam Mar 09 '17 at 12:22
  • @Flimm sory for old topic, but why you say that we cannot count on realpath? Can you explain this? Realpath return absolute path without any trash, so why checking two realpath strings can be dangerous? – Daimos Jun 25 '17 at 16:48
  • @Daimos What if `$path` is equal to `"../../etc/passwd"`? In that case, `realpath("sample/$path")` could resolve to `/etc/passwd`, which you really don't want to happen for security purposes. This is known as a directory traversal attack, see the link in my previous comment. – Flimm Jun 26 '17 at 10:48
  • 1
    @Flimm, but when you check both paths you use realpath on them both. So there is no security risk. Yes, $path in your example can point to /etc/passwd, but then you compare it to default path (for example /home/test) and you are sure, that its invalid one. So realpath and strpos will be safe enough – Daimos Jun 26 '17 at 17:22
6
<?php
    // Current path information
    $path = $_GET['path'];
    $vroot = "sample";

    // Validate that the $path is a subfolder of $vroot
    $vroot = realpath($vroot);
    if(substr(realpath($path), 0, strlen($vroot)) != $vroot or !is_dir($path)) {lid!
        exit("Invalid path");
    } else {
       echo "Ah, everything is alright!";
    }
?>
Evan Fosmark
  • 98,895
  • 36
  • 105
  • 117
0

The use of realpath should not change the path, so I use it in the following way:

function checkPath($pathToCheck) {
    global $basepath;
    $fullpath = $basepath.'/'.$pathToCheck;
    if ($fullpath==realpath($fullpath) && is_dir($fullpath)) {
        return $fullpath;
    } else {
        error_die('path not allowed: '.htmlentities($pathToCheck));
    }
}
Marc Wäckerlin
  • 662
  • 6
  • 7