# Security and efficiency for profile picture upload

I am trying to allow users to upload profile pics on my site. Basically I am following this code: Allow users to upload a pic for their profile.

I don't know if the code is secure enough. I am worried that users may upload malicious files.

Also, I am concerned that if there is many users, I will have many pictures in a single folder and it may slow down the retrieving process. I wonder what is the best practice in storing the profile pics. How many photos maximum should there be in a folder for better efficiency?

if(is_uploaded_file($_FILES['image']['tmp_name'])) { uploadImage($_FILES['image']['tmp_name'], 100, 100, "image/users/test2.jpeg");
}


and

function uploadImage($source,$max_width, $max_height,$destination) {

list($width,$height) = getimagesize($source); if ($width > 150 || $height > 150) {$ratioh = $max_height /$height;
$ratiow =$max_width / $width;$ratio = max($ratioh,$ratiow);
// New dimensions
$newwidth = intval($ratio * $width);$newheight = intval($ratio *$height);

$newImage = imagecreatetruecolor($newwidth, $newheight);$ext = trim(strtolower($_FILES['image']['type']));$sourceImage = null;

// Generate source image depending on file type
switch ($ext) { case "image/jpg": case "image/jpeg":$sourceImage = imagecreatefromjpeg($source); break; case "image/gif":$sourceImage = imagecreatefromgif($source); break; case "image/png":$sourceImage = imagecreatefrompng($source); break; } imagecopyresampled($newImage, $sourceImage, 0, 0, 0, 0,$newwidth, $newheight,$width, $height); // Output file depending on type switch ($ext) {
case "image/jpg":
case "image/jpeg":
imagejpeg($newImage,$destination);
break;
case "image/gif":
imagegif($newImage,$destination);
break;
case "image/png":
imagepng($newImage,$destination);
break;
}

// Destroy resources
imagedestroy($newImage); imagedestroy($sourceImage);

}
}


what happens if I upload a .exe file? or even a .txt file? Your application will give errors. Always have a fallback for if some hack0r uploads a non-picture to your application. Use the default: statement in your switch cases for that and hand back an error when they try to upload a non-image or a image you don't support (e.g. svg).

You talk about security and number of files in one directory, why is there a correlation between these two? The only restriction you have on the number of files per directory depends on the Filestructure format.

I would however create a directory per user instead of storing all the images in the same directory.

This won't help with security, but if you want to shorten your code a little, you can replace this

// Generate source image depending on file type
switch ($ext) { case "image/jpg": case "image/jpeg":$sourceImage = imagecreatefromjpeg($source); break; case "image/gif":$sourceImage = imagecreatefromgif($source); break; case "image/png":$sourceImage = imagecreatefrompng($source); break; }  with this $sourceImage = imagecreatefromstring(file_get_contents(\$source));


Also why don't you just store the whole lot as jpg, why is it necessary to save them as different types

• Always make sure there is a default. Especially in a case like this where the program has a chance of breaking if the conditions aren't met. Feb 21, 2014 at 21:21