2

If this is a newby question, forgive me. I have coded a php file uploader. After hearing that hackers could attatch/disguise code to images to infect sites, I got an idea to solve that problem. To convert the upload image to another file format (png) and then to another (jpg) with 70% quality. This caused the malware to become corrupted. The problem is, this total conversion process takes a about 1 minute at top speed. The service I'm making needs to be quick to handle the file uploads so that the users can go about the work. How can I speed up this process? The upload code is below (important variables are blanked).

// upload the file
$status = "...recieving the file...";
move_uploaded_file($_FILES["file"]["tmp_name"], "$folder" . $_FILES["file"]["name"]);
$status = "...processing the file...";

// convert the file to destroy viruses
$filename21 = "$folder" . $_FILES["file"]["name"];
imagepng(imagecreatefromstring(file_get_contents($filename21)), "$folder"."debug.".$picture_newname.".png");
$imageTmp=imagecreatefrompng("$folder"."debug.".$picture_newname.".png");
imagejpeg($imageTmp, "$folder".$picture_newname.".jpg", 90);
imagedestroy($imageTmp);

These are the steps it follows:

  1. Scan database for file with the same name
  2. if file with same name is found, rename the current upload
  3. receive the file
  4. "evaluate" the file (the double conversion process to protect the server)
  5. insert the info into the uploads database

If any other codes are needed (or if i should do some more timing) please let me know. Thanks in advance for your assistance.

Ibu
  • 42,752
  • 13
  • 76
  • 103
mcfish
  • 102
  • 1
  • 10
  • Is checking the mime-type of the file not enough? Just curious since I don't know anything about security. – Gohn67 Feb 07 '14 at 02:21
  • Possibly. But say someone manages to find away to get past that. I think it's smart to make sure that all possible threats are eliminated. But yes, that is also a good idea I will include that too. @Gohn67 – mcfish Feb 07 '14 at 02:27
  • Maybe it is possible to run anti-virus scan on the image files? – Gohn67 Feb 07 '14 at 02:29
  • True. Do you have suggestions on how I could go about that? I would need to trigger a scan after every upload. @Gohn67 – mcfish Feb 07 '14 at 03:33
  • Unfortunately I have no suggestions. Security is an area where I have zero knowledge. – Gohn67 Feb 07 '14 at 03:34
  • Okay. Thank you for your help. I will research on it. Meanwhile, do you know how to launch another window from php? This way, the server could handle the conversion in the background as the other processes go on. This would drastically improve the upload time, don't you agree? @Gohn67 – mcfish Feb 07 '14 at 03:37

2 Answers2

2

This is a crazy idea. You're not just tying up the server in converting between image formats, you're also degrading the quality of every uploaded image.

I'd recommend a different approach

  1. When a file is uploaded, use PHP's getimagesize() function to check the image. If this function returns FALSE (or an unexpected image type, or strange dimensions, etc.), then the file is corrupt and can be deleted.

  2. Use exiftool or something similar to strip away all the metadata from the uploaded file before you store it away on the server. That way you can ensure that the file only contains image data.

  3. Perhaps you could check that the value of $_FILES["file"]["name"] doesn't contain anything sneaky like ../../ before you use it to save the file on your server.

r3mainer
  • 23,981
  • 3
  • 51
  • 88
1

It's totally bad idea implement double conversion for security purpose, because of DoS attack. Balanced solution between speed & security must contain:

  1. Check MIME type.
  2. Check file extension.
  3. Check file size. (highly recommended)
  4. Check image size. (optional, depends on application requirements)

Something like this:

$allowable_types = array(
    'png'  => 'image/png',
    'jpeg' => 'image/jpeg',
    'gif'  => 'image/gif'
);
$max_file_size = 10240; // 10K limit example

$finfo = new finfo(FILEINFO_MIME_TYPE);
$type  = $finfo->file($_FILES['name']['tmp_name']);
$size  = filesize($_FILES['name']['tmp_name']);
$info  = pathinfo($_FILES['name']['tmp_name']); 

if (isset($allowable_types[$info['extension']])
    and $type === $allowable_types[$info['extension']] 
    and $size <= $max_file_size
) {
    // check image size if your app require this
    move_uploaded_file($_FILES['name']['tmp_name'], $destination);
}

Update

I would not recommend to use $_FILES['file']['name'] in destination path or scan whole directory for same name. Because of some security flaws and performance drop. Better solution is to generate unique name for each image:

$new_name = uniquid() . '.' . $info['extension'];
$destination = $upload_path . $new_name;
Alexander Yancharuk
  • 13,817
  • 5
  • 55
  • 55
  • Thank you. If you checked carefully I do have a MAX file cap. It's about 1 or two KB over the default Android photo size. I will implement the other features you mentioned and remove the double conversion. – mcfish Feb 07 '14 at 04:43
  • @Lite20 Sorry I can't find any mentions of MAX file cap not in your question neither in your comments. – Alexander Yancharuk Feb 07 '14 at 07:34
  • Oh, okay. Well I have it implemented now. Thank's for your assistance. – mcfish Feb 07 '14 at 14:09