views:

93

answers:

6

Hi is this totally safe or not? I would like a totally safe fileuploading script for my new project. Thanks in advice. Here is the one i found:

<?php
if ((($_FILES["file"]["type"] == "image/gif")
|| ($_FILES["file"]["type"] == "image/jpeg")
|| ($_FILES["file"]["type"] == "image/pjpeg"))
&& ($_FILES["file"]["size"] < 20000))
  {
  if ($_FILES["file"]["error"] > 0)
    {
    echo "Return Code: " . $_FILES["file"]["error"] . "<br />";
    }
  else
    {
    echo "Upload: " . $_FILES["file"]["name"] . "<br />";
    echo "Type: " . $_FILES["file"]["type"] . "<br />";
    echo "Size: " . ($_FILES["file"]["size"] / 1024) . " Kb<br />";
    echo "Temp file: " . $_FILES["file"]["tmp_name"] . "<br />";

    if (file_exists("upload/" . $_FILES["file"]["name"]))
      {
      echo $_FILES["file"]["name"] . " already exists. ";
      }
    else
      {
      move_uploaded_file($_FILES["file"]["tmp_name"],
      "upload/" . $_FILES["file"]["name"]);
      echo "Stored in: " . "upload/" . $_FILES["file"]["name"];
      }
    }
  }
else
  {
  echo "Invalid file";
  }
?>
A: 

The files with viruses are still going to get through.

lance
best to check for "goatse.jpg" too
nickf
How can it get trough? nickf said it looks OK.
Come to think of it, I'm not so sure if that script is totally safe or not.
lance
A: 

Looks ok to me.

nickf
So it IS safe? So persons can't upload vulnerable scripts etc on my server?
A: 

Unless files are scanned, there is no totally 100% safe way to do file uploading. Remember, PHP is a scripting language, not a security program language. So it is as safe as you make it. That said, besides limiting the size and type, the only other thing you might be able to do is check for weird file names - weird being what you define for your application.

If you can use a third party service to scan your files (as yahoo scans attatchments for viruses) for virus, that would make the application better, but I don't know of any off the top of my head.

Robert DeBoer
+3  A: 

You can't trust the $_FILES["file"]["type"] for mime types. That information is sent by the browser so it could be faked. It's best to check mime types with mime_content_type or Fileinfo.

smack0007
Or at the very least make sure the file extension is that of an image. That way your web sever will handle it as an image. Won't protect against corrupt or invalid images but it will stop people from uploading runnable PHP scripts.
MitMaro
A: 

This might be the over-pedantic ramblings of a security analyst, but you should validate all those fields, using regular expressions for example. And the content as well. Otherwise you just can't be sure. One could quite easily trick that code into accepting a file as being an 'image/gif' then embed a script in it that a particular Apache handler might choose to treat as a valid command! (maybe not by default, but when you install a 'cool new Apache module' in the future...)

If you want a 'totally' secure script: Validate EVERYTHING to make sure everything is exactly how you want it and nothing more or less.

Rushyo
+2  A: 

One thing to be aware of is MIME Type Detection in Internet Explorer, which can turn your images into a security risk.

Even if a file has the right extension, and is served with an image mime type, if the file itself contains tokens like <html>, <body>, etc, IE (7 and older) may still interpret it as HTML, opening up a possible XSS exploit.

Daniel LeCheminant