tags:

views:

62

answers:

5

I have a php script which takes a relative pathname via $_GET, reads that file and creates a thumbnail of it. I dont want the user to be able to read any file from the server. Only files from a certain directory should be allowed, otherwiese the script should exit().

Here is my folder structure:

files/ <-- all files from this folder are public
my_stuff/ <-- this is the folder of my script that reads the files

My script is accessed via mydomain.com/my_stuff/script.php?pathname=files/some.jpg. What should not be allowed e. g.: mydomain.com/my_stuff/script.php?pathname=files/../db_login.php

So, here is the relevant part of the script in my_stuff folder:

...
$pathname = $_GET['pathname'];
$pathname = realpath('../' . $_GET['pathname']); 

if(strpos($pathname, '/files/') === false) exit('Error');
...

I am not really sure about that approach, doesnt seem too safe for me. Anyone with a better idea?

+2  A: 

I would do a realpath() first (resolving any "../" and other references) and then check whether the result is a sub-directory of the allowed one.

Taking your scenario (I don't understand why you need to use files/ in this case if it's the only directory that is allowed, but anyway):

$basedir = "/etc/www"; 
$allowed = "/etc/www/files";

$pathname = realpath($basedir."/".$_GET["pathname"]);

if (!$pathname) 
 die ("Unknown file path");

// Check whether $pathname begins with $allowed (= is a sub-directory)
if (substring($pathname, 0, strlen($allowed)) != $allowed)
 die ("illegal access!");

As far as I can see, this should be a safe approach.

Pekka
any idea how to set $basedir and $allowed programmatically so that the script is portable between different servers with different paths?
Max
@Max ahh welcome to my answer :)
Col. Shrapnel
A: 

I've made it once like this
basedir is defined as a directory where the script resides.

$basedir=dirname(__FILE__)."/";
$systemdir=realpath($_SERVER['DOCUMENT_ROOT'].$dir)."/";
if (substr($systemdir,0,strlen($basedir)) !== $basedir) {

...but Pekka was faster :)

Col. Shrapnel
The only thing to be careful of with this, is that different implementations (FastCGI, mod_php, Front end server, etc) treat DOCUMENT_ROOT differently. If this is a one-off script, it may not be an issue. But if it's something others are going to use, don't rely on it...
ircmaxell
Good point I keep forgetting it
Col. Shrapnel
A: 

What about this (it handles the case when you move servers, as there are no hardcoded absolute paths):

$pathname = realpath('../' . $_GET['pathname']);
$rootpath = realpath('../files/');

if (strpos($pathname, $rootpath) !== 0) exit('Error');
ircmaxell
A: 

I would also check the presence of ./ or ../ in the $_GET['pathname'], since those would be an almost clear attempt to go where they shouldn't go. Indeed realpath makes it safe (since .. and . are "resolved"), but the strpos checking could be weak in some (maybe buggy) scenario where it is possible that the /files/ string appears later but would be ignored when the actual file is accessed, e.g. an hypothetical tricks like

/site/my_stuf/secretfile.txt /files/

(using such a string to access a file should "see" that "secretfile.txt " (space included) is not a dir name and everything should fail... but as said, this hypothesis could be real on a buggy scenario, and if the tricks exploit some other less hypothetical bug... which of course is not known util it is exploited)

So I repeat my suggestion in particular to check for ../ in the GET string; and, why do not interpret this string as a path starting from files, i.e. prepending files/ to whatever the user supply, and avoiding the presence of ../ as already said?

I don't know if all this is paranoic (or better solution exists), but I've used a similar approach for a url based directory/files lister which allows "direct" access to a subdirectory of a site of mine, and I wanted of course to be sure there were no way to escape from that directory.

ShinTakezou
A: 

NB. The answers provided using realpath are ok, however they should mention that realpath does not work the same across all server variants.

buggedcom