Security: path from $_GET

NogDog

Just want a little sanity check. I'm working on an image manipulation script (watermarking and thumbnails) which gets the file to be processed from the URL ($GET['file']). I'm using the following check to verify that the file path is a .jp[e]g file that is (a) not above the script's directory, (b) not a url, and (c) has a ".jp[e]g" suffix. I'm just wondering if anyone can think of any other nasty thing I should check for that a hacker might want to stick in there should he be aware of the script's URL?

$file = $_GET['file'];
if (preg_match('#(^/|\.\.|(f|ht)tps?://)#i', $file) or 
   !preg_match('#\.jpe?g$#i', $file))
{
   hdr404(); // disallowed path, outta here...
}

(hdr404() simply sends a 404 header and exits.)

laserlight

Wouldn't it be easier and safer to define what is a valid path, and then check if the path is not a valid path? After all, it would just be the usual security mantra of "deny all by default, allow only those deemed necessary" (pardon my paraphrase).

NogDog

The issue is that the environment will be sort of dynamic, in that the script needs to handle any request for a .jpg file as long as it is in a sub-directory from where the script is located, and the user can add sub-directories as needed. After thinking about it a bit, I came up with the following, which I think is a bit more precise:

/**
 * Validate file is .jp[e]g and is not in a directory above this script
 * @return bool
 * @param string $file
 */
function validPath($file)
{
   $thisPath = dirname(realpath( __FILE__ ));
   $filePath = realpath($file);
   $regexp = '/^'.preg_quote($thisPath.DIRECTORY_SEPARATOR).'.*\.jpe?g$/i';
   return preg_match($regexp, $filePath);
}

Weedpacket

Maybe make it a little more generic by splitting apart the "is in subdirectory" and "is JPEG" tasks. Fewer moving parts, y'see.

function is_subdir_of($superFile, $subFile)
{
	$superPath = realpath($superFile);
	$subPath = realpath($subFile);
	if($superPath===false || $subPath===false) return false;

$superDir = dirname($superPath);
$subDir = dirname($subPath);
return strpos($subDir, $superDir)===0;
// substr($subDir, 0, strlen($superDir))==$superDir fails faster, but is uglier
}

if(is_subdir_of(__FILE__, $file))
{
//...
}