Vulnerability in Thumbnail script?

[invision2]

Hi,

I'm using the PHP Thumbnailer Class(http://www.gen-x-design.com/projects...bnailer-class/) on one of my web sites.

However, I wonder if there is a vulnerability in the code.

?php
/**
 * show_image.php
 * 
 * Example utility file for dynamically displaying images
 * 
 * @author      Ian Selby
 * @version     1.0 (php 4 version)
 */

//reference thumbnail class
include_once('thumbnail.inc.php');

$thumb = new Thumbnail($_GET['filename']);
//$thumb->resize($_GET['width'],$_GET['height']);
$thumb->resize(150,150);
$thumb->cropFromCenter(111);
$thumb->show();
$thumb->destruct();
exit;
?> 

It works like show_image.php?filename=http://www.mysite.com/bigimage.jpg

However, is this part secure?

$thumb = new Thumbnail($_GET['filename']); 

If not, how would I best fix this so it only accepts files on my server?

It doesn't seem to filter or santise my $_GET value.


Many thanks for your thoughts.

[frank1]

taking filename though get is always considered to be "dont do it" practice...as far as i know...
in this script lots depends upon
'thumbnail.inc.php'
how it is filtering type of file it accepts

other problem (may be you are concerned) may be bandwidth theft,if somebody spots this file then they might use it as image resize and your bandwidth will be used...or launch an attack....
when we think in small scale,it may not sound that bad...
but if your competitor loops 1000 images to your file then problem will start...

one solution i can think of,
why dont you make it to accept relative path only rather than absolute
next solution might be to check base_url there are function in php for that....
thought not 100% safe they can be useful...

[invision2]

I think bandwidth theft may be an issue actually. Thank you for raising this.

why dont you make it to accept relative path only rather than absolute
next solution might be to check base_url there are function in php for that....

How would I best do this?
Would I disallow 'http' or 'www' appearing in the $_GET['filename'] ?


Many thanks for your help.

[AnthonySterling]

The library checks if the file exists, however, I'd lock the search down somewhat as the library doesn't.

Something similar to...

if(true === file_exists(sprintf('/var/www/assets/images/%s', basename($_GET['filename'])))){
    #do stuff
} 

Either that, or extend ThumbBase and override the fileExistsAndReadable method to just look in a designated location of your choosing.

[invision2]

Hi Anthony,

Brilliant! That seems perfect. Just tried it out. Loads the local image fine and doesn't load external ones.

Thanks once more.