Is it everything I can do to make this upload form "safe"?

Question

I did the same upload form for photo uploading like here. Is it everything I can do to protect my website or I need to add something? Thank you very much.

Answer 1

I'd say no. There are checks in there for restricting the type of the file being uploaded:

if ((($_FILES["file"]["type"] == "image/gif")
|| ($_FILES["file"]["type"] == "image/jpeg")
|| ($_FILES["file"]["type"] == "image/pjpeg"))
....

That "type" is provided by the browser and can't be relied on for security purposes. Someone could easily hack something together that sent an executable file with a type of "image/gif" and the script would happily accept it.

A better check would be to use something like getimagesize or one of the other GD functions to verify that it is actually an image.

Answer 2

i have this old function that i still use for creating single image :

    <?
    $portal_name = 'yoursite name that will be written as watermark';

     /**
     *
     * @param $only_file_name if isset, returns two files as array with paths to folder where they are saved
     * @param $type_action if isset crop, crops the images
     * @param $t_h = thumbnail height
     * @param $t_w = thumbnail width
     * @param $n_h = big height
     * @param $n_w = big width
     * @param $path1
     * @param $path2
     * @param $param_file_name = name your file, e.g. rand(0,50); or better time();
     * @param $file_object = the $_FILES['filename'];
     * @param $file_size = file size in kb
     * @param $thumb = shall i crop the thumbnail ?
     * @param $watermarkon = use watermark or not
     */


    function Make_Single_Picture($only_file_name="on",$type_action="crop", $t_h, $t_w, $n_h, $n_w, $path1, $path2, $param_file_name, $file_object, $file_size, $thumb="crop", $watermarkon="yes") {
        global $portal_name;



        $Picture=$file_object;



        $errors=0;


        $image =$Picture["name"];
        $uploadedfile = $Picture['tmp_name'];

        $watermark = imagecreatefrompng("watermark.png");
        imagealphablending($watermark, true);

        $watermark_width  = imagesx($watermark);
        $watermark_height = imagesy($watermark);



        if ($image)
        {
            $filename = stripslashes($Picture['name']);
            $extension = getExtension($filename);
            $extension = strtolower($extension);
            if (($extension != "jpg") && ($extension != "jpeg")

            && ($extension != "png"))
            {

                return FALSE;

                $errors=1;
            }
            else
            {
                $size=filesize($Picture['tmp_name']);

                if ($size > $file_size*1024)
                {
                    return FALSE;

                    $errors=1;
                }

                if($extension=="jpg" || $extension=="jpeg" )
                {
                    $uploadedfile = $Picture['tmp_name'];
                    $src = imagecreatefromjpeg($uploadedfile);
                }
                else if($extension=="png" || $extension=="gif")
                {
                    $uploadedfile = $Picture['tmp_name'];
                    $src = imagecreatefrompng($uploadedfile);
                }
                else
                {
                    $src = imagecreatefromgif($uploadedfile);
                }

                list($width,$height)=getimagesize($uploadedfile);

                if ($type_action=="crop") {
                    $newwidth=$n_w;
                    $newheight=$n_h;
                    if ($width<$n_w) {
                        $newwidth=$width;
                    }
                    if ($width<$n_h) {
                        $newheight=$height;
                    }
                    $tmp=imagecreatetruecolor($newwidth,$newheight);

                }
                else {

                    $newwidth=$n_w;
                    $newheight=($height/$width)*$newwidth;
                    if ($width<$n_w) {
                        $newwidth=$width;
                    }
                    if ($width<$n_h) {
                        $newheight=$height;
                    }
                    $tmp=imagecreatetruecolor($newwidth,$newheight);


                }


                if ($thumb=="crop") {
                    $newwidth1=$t_w;
                    $newheight1=$t_h; 
                    $tmp1=imagecreatetruecolor($newwidth1,$newheight1);
                }
                else {$newwidth1=$t_w;
                $newheight1= ($height/$width)*$newwidth1;
                $tmp1=imagecreatetruecolor($newwidth1,$newheight1);}


                imagecopyresampled($tmp,$src,0,0,0,0,$newwidth,$newheight, $width,$height);
                $dest_x = ($newwidth - $watermark_width) + 20;
                $dest_y = ($newheight - $watermark_height) + 35;
                if ($watermarkon=="yes") {
                $color_of_the_text = imagecolorallocate($tmp, 255, 255, 255);

                // path to the font that you want to use when printing watermark
                $font = "txt_cache/GILLUBCD.TTF";
                imagettftext($tmp, 16, 0, $dest_x, $dest_y, $color_of_the_text, $font, $portal_name);
                }
                imagecopyresampled($tmp1,$src,0,0,0,0,$newwidth1,$newheight1, $width, $height);
                $time_and_id = ''.$param_file_name.'_'.$kolko.'';
                $image_name=$time_and_id.'.'.$extension;

                $filename = "$path1". $image_name;
                $filename1 = "$path2". $image_name;

                imagejpeg($tmp,$filename,100);
                imagejpeg($tmp1,$filename1,100);

                imagedestroy($src);
                imagedestroy($tmp);
                imagedestroy($tmp1);
                imagedestroy($watermark);

                if ($only_file_name=="on") {
                    return array('huge'=>$image_name, 'thumb'=>$image_name);

                } else {
                    return array('huge'=>$filename, 'thumb'=>$filename1);
                }

            }
        }



    }


    /**
 *
 * @param entire array of files $files to be used BEFORE foreach
 * @return will return a valid array of files.
 * @author vertazzar
 */
 function fixFilesArray(&$files)
{
    $names = array( 'name' => 1, 'type' => 1, 'tmp_name' => 1, 'error' => 1, 'size' => 1);

    foreach ($files as $key => $part) {
         $key = (string) $key;
        if (isset($names[$key]) && is_array($part)) {
            foreach ($part as $position => $value) {
                $files[$position][$key] = $value;
            }

            unset($files[$key]);
        }
    }
}


    function getExtension($str) {

        $i = strrpos($str,".");
        if (!$i) { return ""; }

        $l = strlen($str) - $i;
        $ext = substr($str,$i+1,$l);
        return $ext;
    }

    ?>

usage :

$files = Make_Single_Picture('off', 'crop', 200,100,800,600,'path/big/', 'path/small', time(), $_FILES['filename'], 1000, 'crop', 'yes');
<? echo $files['huge']; echo $files['thumb']; ?>

NOTE: this function/code is slightly changed from original version, because it variables were on other language, so you would harder understand what is what, so you might want to test closely first.