PHP image upload security check list

usef_ksa picture usef_ksa · Nov 12, 2010 · Viewed 52.6k times · Source

I am programming a script to upload images to my application. Are the following security steps enough to make the application safe from the script side?

  • Disable PHP from running inside the upload folder using .httaccess.
  • Do not allow upload if the file name contains string "php".
  • Allow only extensions: jpg,jpeg,gif and png.
  • Allow only image file type.
  • Disallow image with two file type.
  • Change the image name.
  • Upload to a sub-directory not root directory.

This is my script:

 $filename=$_FILES['my_files']['name'];
 $filetype=$_FILES['my_files']['type'];
 $filename = strtolower($filename);
 $filetype = strtolower($filetype);

 //check if contain php and kill it 
 $pos = strpos($filename,'php');
 if(!($pos === false)) {
  die('error');
 }




 //get the file ext

 $file_ext = strrchr($filename, '.');


 //check if its allowed or not
 $whitelist = array(".jpg",".jpeg",".gif",".png"); 
 if (!(in_array($file_ext, $whitelist))) {
    die('not allowed extension,please upload images only');
 }


 //check upload type
 $pos = strpos($filetype,'image');
 if($pos === false) {
  die('error 1');
 }
 $imageinfo = getimagesize($_FILES['my_files']['tmp_name']);
 if($imageinfo['mime'] != 'image/gif' && $imageinfo['mime'] != 'image/jpeg'&& $imageinfo['mime']      != 'image/jpg'&& $imageinfo['mime'] != 'image/png') {
   die('error 2');
 }
//check double file type (image with comment)
if(substr_count($filetype, '/')>1){
die('error 3')
}

 // upload to upload direcory 
 $uploaddir = 'upload/'.date("Y-m-d").'/' ;

if (file_exists($uploaddir)) {  
} else {  
    mkdir( $uploaddir, 0777);  
}  
  //change the image name
 $uploadfile = $uploaddir . md5(basename($_FILES['my_files']['name'])).$file_ext;



  if (move_uploaded_file($_FILES['my_files']['tmp_name'], $uploadfile)) {
 echo "<img id=\"upload_id\" src=\"".$uploadfile."\"><br />";
  } else {
   echo "error";
  }

Any new tips are welcome :)

Answer

Halil &#214;zg&#252;r picture Halil Özgür · Nov 12, 2010

Re-process the image using GD (or Imagick) and save the processed image. All others are just fun boring for hackers.

Edit: And as rr pointed out, use move_uploaded_file() for any upload.

Late Edit: By the way, you'd want to be very restrictive about your upload folder. Those places are one of the dark corners where many exploits happen. This is valid for any type of upload and any programming language/server. Check https://www.owasp.org/index.php/Unrestricted_File_Upload