How secure is this image upload?

[tomhoad]

Hi,

 

having had a look around these forums and the net, I came across this script at http://www.webcheatsheet.com/php/file_upload.php for uploading images securely.

 

I'm just interested to see how secure you think this is:

 

<?php
//Сheck that we have a file
if((!empty($_FILES["uploaded_file"])) && ($_FILES['uploaded_file']['error'] == 0)) {
  //Check if the file is JPEG image and it's size is less than 350Kb
  $filename = basename($_FILES['uploaded_file']['name']);
  $ext = substr($filename, strrpos($filename, '.') + 1);
  if (($ext == "jpg") && ($_FILES["uploaded_file"]["type"] == "image/jpeg") && 
    ($_FILES["uploaded_file"]["size"] < 350000)) {
    //Determine the path to which we want to save this file
      $newname = dirname(__FILE__).'/upload/'.$filename;
      //Check if the file with the same name is already exists on the server
      if (!file_exists($newname)) {
        //Attempt to move the uploaded file to it's new place
        if ((move_uploaded_file($_FILES['uploaded_file']['tmp_name'],$newname))) {
           echo "It's done! The file has been saved as: ".$newname;
        } else {
           echo "Error: A problem occurred during file upload!";
        }
      } else {
         echo "Error: File ".$_FILES["uploaded_file"]["name"]." already exists";
      }
  } else {
     echo "Error: Only .jpg images under 350Kb are accepted for upload";
  }
} else {
echo "Error: No file uploaded";
}
?>

 

It seems pretty good, I can't upload big files or files without a .jpg extension, and even if I change a .php file (for example) to a .jpg, it still recognises it as a .php and rejects it.

 

What do you think - I want to implement it on a site but just want to double check it with people who know far more than me about these things 

[jackpf]

Seems ok. One thing I also use when allowing people to upload files is to check for characters such as / \ ' " etc as they are often deemed "prohibited" filenames by *nix and are impossible to delete over ftp.

 

I'd either do a check for this, or remove it from the file name. Apart from that, code seems secure as anything is.