Getting a local file with file_get_contents

Question

my url is like page.php?path=content/x/y/z/aaa.md. Is the following php code secure?

include "Parsedown.php";

function path_purifier($path) {
  if(substr($path, 0, 8) !== "content/")
    return null;
  if (strpos($path,'..') !== false)
    return null;
  return "./" . $path;
}

$parsedown = new Parsedown();
$path = $_GET['path'];
$path = path_purifier($path);
echo $parsedown->text(file_get_contents($path));

Answer 1

Difficult to say.

It's unclear what the $parsedown->text() method does, nor do we know what files are under content/ and where they come from.

We cannot know, if this is 'secure'. Define what you mean by 'secure'?

What if $parsedown->text() receives FALSE? What does it do? Something 'unsecure'?

Who controls what files that are present under content/? The user of your site? You?

But if your question is: "Does this code restrict file access to only the files under content/?", then I think the answer should be: "Yes." You check that the beginning is content/ and you prevent going to the parent directory. As far as I can tell that should be enough.

I would prefer a file_exists() check, just so you know there's something there before you go and process the, non-existent, content. And possibly I would do a check on the file content, if possible.

I would also really like to see the usage of an absolute path here, just to be safe. Something like:

define('THIS_ROOT','/this/is/my/file/root/');

function path_purifier($path) 
{
  if ((substr($path,0,8) == 'content/') &&
      (strpos($path,'..') === false)) return THIS_ROOT.$path;
                                 else return NULL;
}

But again, this all depends on what your code actually does.

You might want to further restrict the characters in the path to those that are valid. To get rid of unwanted stuff.

$path = preg_replace('/[^A-Za-z0-9.\/]/', '', $path);

this is just an example, you might need another regular expression for your files.

You might consider not giving the paths away like this at all. You can store the paths in a database and refer to the entries in the database by an ID.

You can further secure those ID's by specifying, in the database, which users are allowed to use which paths.

You can encrypt the ID's themselves so it will be virtually impossible for someone to change them. Your URL would then look something like this:

page.php?id=cVIyWjJwMmFrZG1k

Change anything and you would know about it.

Answer 2

As @KIKO stated, this is a bit dangerous to use relative paths (here, contents/ which is ./contents) because if your php script becomes included or reused, relative path (.) may points to a different location.

You can create a constant or put that absolute path in a config file. At the end, you'll check that the absolute path given by user is beginning by the expected absolute path:

define('THIS_ROOT','/this/is/my/file/root/');

function path_check($path) {
   return strncmp(THIS_ROOT, realpath($path), strlen(THIS_ROOT)) == 0;
}

If the given $path does not exist, realpath will return false so this check manages both security and resource existence.