Check secure image upload

Question

I want to upload images to server from browser window. I'm using this function for image upload.Images is uploaded by GD library.
What is your opinion of this code?
I want to know if there is still a way to upload a malicious file?

<?php
function imageUpload($name, $thumb = false, $max_size = 5242880, $height = null, $width = null, $T__height = 100, $T__width = 100)
{
    if(!isset($_FILES[$name]))
    {
        return false;
    }

    $allowedTypes = array('image/gif', 'image/jpeg', 'image/png', 'image/wbmp');
    $image = &$_FILES[$name];

    if ($image['error'] == 0 && in_array($image['type'], $allowedTypes) && $image['size'] <= $max_size)
    {
        $in = '';
        switch ($image['type'])
        {
            case 'image/gif':
                $in = 'imagecreatefromgif';
                break;
            case 'image/jpeg':
                $in = 'imagecreatefromjpeg';
                break;
            case 'image/png':
                $in = 'imagecreatefrompng';
                break;
            case 'image/wbmp':
                $in = 'imagecreatefromwbmp';
                break;
        }

        if ($in == '')
        {
            return false;
        }

        $src = $in($image['tmp_name']);
        $height = ($height == null || $height <= 0 ? imagesy($src) : $height);
        $width = ($width == null || $width <= 0 ? imagesx($src) : $width);

        $dst = imagecreatetruecolor($width, $height);
        imagecopyresized($dst, $src, 0, 0, 0, 0, $width, $height, imagesx($src), imagesy($src));

        $fileName = '';
        do
        {
            $fileName = makeHash(mt_rand(0, mt_getrandmax()) . microtime(true), $image['tmp_name']);
        }
        while(file_exists('/image/' . $fileName . '.jpg') || ($thumb && file_exists('/thumb/' . $fileName . '.jpg')));
        if ($fileName == '')
        {
            return false;
        }

        imagejpeg($dst, '/image/' . $fileName . '.jpg', 75);

        if ($thumb)
        {
            $dst = imagecreatetruecolor($T__width, $T__height);
            imagecopyresized($dst, $src, 0, 0, 0, 0, $T__width, $T__height, imagesx($src), imagesy($src));
            imagejpeg($dst, '/thumb/' . $fileName . '.jpg', 75);
        }

        imagedestroy($src);
        imagedestroy($dst);

        return $fileName . '.jpg';
    }
}
?>

I'm voting to close this question as off-topic because it fits on http://codereview.stackexchange.com, not Stack Overflow. — deceze, Jan 25 '15 at 14:26

score 1 · Accepted Answer · answered Jan 25 '15 at 14:34

First of all the mimetypes shouldn't be relied upon for any kind of security mechanisms since an attacker has full control over that field (they can upload a .php file with a type of image/gif that will pass those checks). Then if they include a null byte in the file name %00 or \00 then it is possible to bypass the condition that adds a .jpg extention to the image like this evil.php\00.jpg so by the time the file hits the disk it will just be evil.php. It's hard to evaluate your code fully without seeing all of the functions though because depending on what they all do might open up vulnerabilities. (for example it's not clear where they files are being stored, what is calling them, loading them, etc).

Check secure image upload

1 Answers1