CODEWITHSUNDEEP
phpsecurity
Jake Petroules
Jake Petroules

Reputation: 24160

How to use file_get_contents safely?

I am making a basic PHP source viewer for my blog's example code folder.

<?php

if (isset($_GET['file']))
{
    header('Content-type: text/plain');
    $filename = realpath($_GET['file']);
    if (startsWith($filename, dirname(__FILE__)))
    {
        echo file_get_contents($filename);
    }
}

function startsWith($haystack, $needle)
{
    $length = strlen($needle);
    return (substr($haystack, 0, $length) === $needle);
}

?>

Is what I have here sufficient that it will never allow a file outside the directory in which this script is located, or subdirectories of this script's directory, to be viewed? I'm guessing there's a better solution than startsWith too, for checking whether a path is a descendant of a particular directory?

Upvotes: 3

Views: 3900

Answers (4)

Lawrence Cherone
Lawrence Cherone

Reputation: 46610

If I was to approach a project like this I would:

Use a combination of your solution & htaccess to mask the original filename as having:

http://example.com/viewsource.php?file=./project1/index.php

would be just too obvious and tempting to mess around with, using htaccess you could have:

http://example.com/source/project1/index.php

that points too

h1ddEnfIlEs/viewSource/index.php?file=$1&folder=$2

RewriteRule ^source/([a-z0-9]+)/([a-z0-9]+).php$ h1ddEnfIlEs/viewSourceScript/index.php?file=$2&project=$1

Then with the source viewer h1ddEnfIlEs/viewSourceScript/index.php 

<?php 
if(
is_dir('./h1ddEnfIlEs/'.$_GET['project'].'/')===true && 
is_file('./h1ddEnfIlEs/'.$_GET['project'].'/'.basename($_GET['file']))
){
    highlight_file('./h1ddEnfIlEs/'.$_GET['project'].'/'.basename($_GET['file']));
}
?>

Upvotes: 0

Sid
Sid

Reputation: 2156

As an aside, look into using strncmp instead of your custom startsWith function.

Upvotes: 0

feeela
feeela

Reputation: 29932

You maybe reduce the input to a filename (see finfo) and specify the directory to search in, if it's the same directory every time - so you don't need to check via startsWith().

If you only want to show PHP-sourcecode, try to enable *.phps-file-handling within your webserver. See: https://serverfault.com/questions/62410/apache-htaccess-phps

Upvotes: 0

Jon
Jon

Reputation: 437444

It's going to be safe, yes. The realpath part is what you have to do, and you are doing it. This code does what it's supposed to just fine.

Upvotes: 3

Related Questions