Skip to content
Advertisement

Secure image upload in php

I am making an image upload function which I can re-use in my code, which has to be 100% secure. Please tell me if you can spot and security holes in my initial code;

function Upload($file)
{
    list($width,$height,$type,$attr) = getimagesize($file);
    $mime = image_type_to_mime_type($type);
    
    if(($mime != "image/jpeg") && ($mime != "image/pjpeg") && ($mime != "image/png")) 
    {
        return 'Error3: Upload file type un-recognized. Only .JPG or .PNG images allowed';
    }else{
        $Newname = md5('sillysalt'.time());
        if (move_uploaded_file($file, 'images/'.$Newname.$type)) 
        {
            return 'Uploaded!';
        }else{
            return 'Server Error!';
        } 
    }
}

UPDATE This is how far I’ve gotten with your help and some research, please tell me what you think. I don’t mind much about the speed, for me it’s all about being 100% secure, or as close to.

function Upload($file)
{
    list($width,$height,$type,$attr) = getimagesize($file);
    $mime = image_type_to_mime_type($type);
    $folder = 'images/';
    
    // mime checks add a layer of security that keeps out less sophisticated attackers 
    if(($mime != "image/jpeg") && ($mime != "image/pjpeg") && ($mime != "image/png")) 
    {
        return 'Error3: Upload file type un-recognized. Only .JPG or .PNG images allowed';
    }else{
        // If the file has no width its not a valid image
        if(!$width)
        {
            $Newname = md5('sillysalt'.time());
            $finfo = finfo_open(FILEINFO_MIME_TYPE);
            $mime2 = finfo_file($finfo, $folder.$Newname);
            
            // Should I remove this second mime check? since the info comes form the same spoofable source in the image
            if(($mime != "image/jpeg") && ($mime != "image/pjpeg") && ($mime != "image/png")) 
            {
                $fileType = exif_imagetype($file);
                $allowed = array(IMAGETYPE_JPEG, IMAGETYPE_PNG);
                if(!in_array($fileType, $allowed))
                {
                    // don't overwrite an existing file
                    $i = 0;
                    $parts = pathinfo($file);
                    while(file_exists($folder . $name))
                    {
                        $i++;
                        $name = $Newname."-".$i.".".$parts["extension"];
                    }
    
                    if(move_uploaded_file($file, $folder.$name)) 
                    {
                        // set good permissions for the file
                        chmod($name, 0644);
                        return 'Uploaded!';
                    }else{
                        return 'Server Error!';
                    } 
                }
            }
        }
    }
}

Advertisement

Answer

As long as you don’t use the FileInfo (http://www.php.net/manual/en/ref.fileinfo.php) extensions from php to check the mime type, your function is not secure at all (think later you’ll want to upload pdf’s, excels, etc).
Also, md5 over md5 does nothing than increasing the collision chances.

L.E: Something as simple as the following should do it: 

function getExtensionToMimeTypeMapping() {
    return array(
        'ai'=>'application/postscript',
        'aif'=>'audio/x-aiff',
        'aifc'=>'audio/x-aiff',
        'aiff'=>'audio/x-aiff',
        'anx'=>'application/annodex',
        'asc'=>'text/plain',
        'au'=>'audio/basic',
        'avi'=>'video/x-msvideo',
        'axa'=>'audio/annodex',
        'axv'=>'video/annodex',
        'bcpio'=>'application/x-bcpio',
        'bin'=>'application/octet-stream',
        'bmp'=>'image/bmp',
        'c'=>'text/plain',
        'cc'=>'text/plain',
        'ccad'=>'application/clariscad',
        'cdf'=>'application/x-netcdf',
        'class'=>'application/octet-stream',
        'cpio'=>'application/x-cpio',
        'cpt'=>'application/mac-compactpro',
        'csh'=>'application/x-csh',
        'css'=>'text/css',
        'csv'=>'text/csv',
        'dcr'=>'application/x-director',
        'dir'=>'application/x-director',
        'dms'=>'application/octet-stream',
        'doc'=>'application/msword',
        'drw'=>'application/drafting',
        'dvi'=>'application/x-dvi',
        'dwg'=>'application/acad',
        'dxf'=>'application/dxf',
        'dxr'=>'application/x-director',
        'eps'=>'application/postscript',
        'etx'=>'text/x-setext',
        'exe'=>'application/octet-stream',
        'ez'=>'application/andrew-inset',
        'f'=>'text/plain',
        'f90'=>'text/plain',
        'flac'=>'audio/flac',
        'fli'=>'video/x-fli',
        'flv'=>'video/x-flv',
        'gif'=>'image/gif',
        'gtar'=>'application/x-gtar',
        'gz'=>'application/x-gzip',
        'h'=>'text/plain',
        'hdf'=>'application/x-hdf',
        'hh'=>'text/plain',
        'hqx'=>'application/mac-binhex40',
        'htm'=>'text/html',
        'html'=>'text/html',
        'ice'=>'x-conference/x-cooltalk',
        'ief'=>'image/ief',
        'iges'=>'model/iges',
        'igs'=>'model/iges',
        'ips'=>'application/x-ipscript',
        'ipx'=>'application/x-ipix',
        'jpe'=>'image/jpeg',
        'jpeg'=>'image/jpeg',
        'jpg'=>'image/jpeg',
        'js'=>'application/x-javascript',
        'kar'=>'audio/midi',
        'latex'=>'application/x-latex',
        'lha'=>'application/octet-stream',
        'lsp'=>'application/x-lisp',
        'lzh'=>'application/octet-stream',
        'm'=>'text/plain',
        'man'=>'application/x-troff-man',
        'me'=>'application/x-troff-me',
        'mesh'=>'model/mesh',
        'mid'=>'audio/midi',
        'midi'=>'audio/midi',
        'mif'=>'application/vnd.mif',
        'mime'=>'www/mime',
        'mov'=>'video/quicktime',
        'movie'=>'video/x-sgi-movie',
        'mp2'=>'audio/mpeg',
        'mp3'=>'audio/mpeg',
        'mpe'=>'video/mpeg',
        'mpeg'=>'video/mpeg',
        'mpg'=>'video/mpeg',
        'mpga'=>'audio/mpeg',
        'ms'=>'application/x-troff-ms',
        'msh'=>'model/mesh',
        'nc'=>'application/x-netcdf',
        'oga'=>'audio/ogg',
        'ogg'=>'audio/ogg',
        'ogv'=>'video/ogg',
        'ogx'=>'application/ogg',
        'oda'=>'application/oda',
        'pbm'=>'image/x-portable-bitmap',
        'pdb'=>'chemical/x-pdb',
        'pdf'=>'application/pdf',
        'pgm'=>'image/x-portable-graymap',
        'pgn'=>'application/x-chess-pgn',
        'png'=>'image/png',
        'pnm'=>'image/x-portable-anymap',
        'pot'=>'application/mspowerpoint',
        'ppm'=>'image/x-portable-pixmap',
        'pps'=>'application/mspowerpoint',
        'ppt'=>'application/mspowerpoint',
        'ppz'=>'application/mspowerpoint',
        'pre'=>'application/x-freelance',
        'prt'=>'application/pro_eng',
        'ps'=>'application/postscript',
        'qt'=>'video/quicktime',
        'ra'=>'audio/x-realaudio',
        'ram'=>'audio/x-pn-realaudio',
        'ras'=>'image/cmu-raster',
        'rgb'=>'image/x-rgb',
        'rm'=>'audio/x-pn-realaudio',
        'roff'=>'application/x-troff',
        'rpm'=>'audio/x-pn-realaudio-plugin',
        'rtf'=>'text/rtf',
        'rtx'=>'text/richtext',
        'scm'=>'application/x-lotusscreencam',
        'set'=>'application/set',
        'sgm'=>'text/sgml',
        'sgml'=>'text/sgml',
        'sh'=>'application/x-sh',
        'shar'=>'application/x-shar',
        'silo'=>'model/mesh',
        'sit'=>'application/x-stuffit',
        'skd'=>'application/x-koan',
        'skm'=>'application/x-koan',
        'skp'=>'application/x-koan',
        'skt'=>'application/x-koan',
        'smi'=>'application/smil',
        'smil'=>'application/smil',
        'snd'=>'audio/basic',
        'sol'=>'application/solids',
        'spl'=>'application/x-futuresplash',
        'spx'=>'audio/ogg',
        'src'=>'application/x-wais-source',
        'step'=>'application/STEP',
        'stl'=>'application/SLA',
        'stp'=>'application/STEP',
        'sv4cpio'=>'application/x-sv4cpio',
        'sv4crc'=>'application/x-sv4crc',
        'swf'=>'application/x-shockwave-flash',
        't'=>'application/x-troff',
        'tar'=>'application/x-tar',
        'tcl'=>'application/x-tcl',
        'tex'=>'application/x-tex',
        'texi'=>'application/x-texinfo',
        'texinfo'=>'application/x-texinfo',
        'tif'=>'image/tiff',
        'tiff'=>'image/tiff',
        'tr'=>'application/x-troff',
        'tsi'=>'audio/TSP-audio',
        'tsp'=>'application/dsptype',
        'tsv'=>'text/tab-separated-values',
        'txt'=>'text/plain',
        'unv'=>'application/i-deas',
        'ustar'=>'application/x-ustar',
        'vcd'=>'application/x-cdlink',
        'vda'=>'application/vda',
        'viv'=>'video/vnd.vivo',
        'vivo'=>'video/vnd.vivo',
        'vrml'=>'model/vrml',
        'wav'=>'audio/x-wav',
        'wrl'=>'model/vrml',
        'xbm'=>'image/x-xbitmap',
        'xlc'=>'application/vnd.ms-excel',
        'xll'=>'application/vnd.ms-excel',
        'xlm'=>'application/vnd.ms-excel',
        'xls'=>'application/vnd.ms-excel',
        'xlw'=>'application/vnd.ms-excel',
        'xml'=>'application/xml',
        'xpm'=>'image/x-xpixmap',
        'xspf'=>'application/xspf+xml',
        'xwd'=>'image/x-xwindowdump',
        'xyz'=>'chemical/x-pdb',
        'zip'=>'application/zip',
    );
}

function getMimeType($filePath) {

    if (!is_file($filePath)) {
        return false;
    }

    $finfo = finfo_open(FILEINFO_MIME_TYPE);
    $mime = finfo_file($finfo, $filePath);
    finfo_close($finfo);

    return $mime;
}

function upload($filePath, $destinationDir = 'images', array $allowedMimes = array()) {

    if (!is_file($filePath) || !is_dir($destinationDir)) {
        return false;
    }

    if (!($mime = getMimeType($filePath))) {
        return false;
    }

    if (!in_array($mime, $allowedMimes)) {
        return false;
    }

    $ext = null;
    $extMapping = getExtensionToMimeTypeMapping();
    foreach ($extMapping as $extension => $mimeType) {
        if ($mimeType == $mime) {
            $ext = $extension;
            break;
        }
    }

    if (empty($ext)) {
        $ext = pathinfo($filePath, PATHINFO_EXTENSION);
    }

    if (empty($ext)) {
        return false;
    }

    $fileName = md5(uniqid(rand(0, time()), true)) . '.' . $ext;
    $newFilePath = $destinationDir.'/'.$fileName;

    if(!rename($filePath, $newFilePath)) {
        return false;
    }

    return $fileName;
}

// use it 
if (isset($_FILES['something']['tmp_name'])) {
    $file = $_FILES['something']['tmp_name'];
    $storagePath = 'images'; // this is relative to this script, better use absolute path.
    $allowedMimes = array('image/png', 'image/jpg', 'image/gif', 'image/pjpeg');

    $fileName = upload($file, $storagePath, $allowedMimes);
    if (!$fileName) {
        exit ('Your file type is not allowed.');
    } else {
         // check if file is image, optional, in case you allow multiple types of files.
         // $imageInfo = @getimagesize($storagePath.'/'.$fileName);
        exit ("Your uploaded file is {$fileName} and can be found at {$storagePath}/{$fileName}");
    }
}
User contributions licensed under: CC BY-SA
7 People found this is helpful
Advertisement