Is it secure to place uploaded images in a public folder?

Viewed 4440

I've just had a discussion with my teammate about the location of user uploaded images in an image gallery. I would like a broader insight on the methods we suggest.

My teammate wrote a controller + action that calls file_get_contents on an image file placed in a folder that's not available for public browsing (i.e., outside public_html on the server), and echoes it via a header. This is secure, but since we use Zend Framework, it's also crawling slow - each call to the image controller costs us approx 500ms of lag due to the bootstrap's queries being executed. It's annoying since the picture gallery view displays over 20 images at the same time.

In short, the relevant code would be:

class ImageController extends Zend_Controller_Action {
    public function showAction () {
        $filename = addslashes($this->_getParam('filename'));
        if(!is_file($filename)) {
            $filename = APPLICATION_PATH.'/../public/img/nopicture.jpg';
        }
        $this->_helper->viewRenderer->setNoRender(true);
        $this->view->layout()->disableLayout();
        $img = file_get_contents($filename);
        header('Content-Type: image/jpeg');
        $modified = new Zend_Date(filemtime($filename));
        $this->getResponse()
             ->setHeader('Last-Modified',$modified->toString(Zend_Date::RFC_1123))
             ->setHeader('Content-Type', 'image/jpeg')
             ->setHeader('Expires', '', true)
             ->setHeader('Cache-Control', 'public', true)
             ->setHeader('Cache-Control', 'max-age=3800')
             ->setHeader('Pragma', '', true);
        echo $img;
    }
}

Then, in a view, we just call:

<img src="<?php echo $this->url(array('controller' => 'image', 'action' => 'show', 'filename' => PATH_TO_HIDDEN_LOCATION.'/filename.jpg')); ?>" />

I have a different approach: I prefer to keep the original images in a hidden location, but as soon as they are requested, copy them to a public location and provide a link to it (with an extra mechanism, run by cron, to wipe the public images directory every now and then in order not to waste space, and a robots.txt telling Google not to index the directory). The solution places files (a few at every given moment) in a publicly accessible directory (provided one knows the filename), but also requires only a view helper, thus not launching the bootstrap:

class Zend_View_Helper_ShowImage extends Zend_View_Helper_Abstract {
    public function showImage ($filename) {
        if (!file_exists(PUBLIC_PATH."/img/{$filename}")) {
            if (!copy(PATH_TO_HIDDEN_FILES."/{$filename}",PUBLIC_PATH."/img/{$filename}"))
                $url = PUBLIC_PATH.'/img/nopicture.jpg';
            else
                $url = PUBLIC_PATH."/img/{$filename}";
        } else {
            $url = PUBLIC_PATH."/img/{$filename}"
        }
        return "{$url}";
    }
}

With the aid of this helper, the call is very simple in the view:

<img src="<?php echo $this->showImage('filename.jpg'); ?>" />

Question: Does my approach pose a security threat, as my coleague states? What are the potential risks of this? And, most importantly, do the security threats, if any, outweigh the 10 seconds gain on page load?

In case it matters: we're working on a community portal with around 15K registered users, with the galleries being a very frequently used feature.

*The code I pasted is an edited, simplified version of what each of us has come up with - just to show the mechanics of both approaches.

6 Answers
Related