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 :)
For security test of the image files, I can think of 4 level of securities. They would be:
($file_info = getimagesize($_FILES['image_file']; $file_mime = $file_info['mime'];)
Note: Loading entire image would be slow.
XSS Warning
One more very important remark. Do not serve/upload anything that could be interpreted as HTML in the browser.
Since the files are on your domain, javascript contained in that HTML document will have access to all your cookies, enabling some sort of XSS attack.
Attack scenario:
The attacker uploads HTML file with JS code that sends all the cookies to his server.
The attacker sends the link to your users via mail, PM, or simply via iframe on his or any other site.
Most secure solution:
Make uploaded content available only on the subdomain or on the another domain. This way cookies are not going to be accessible. This is also one of the google's performance tips:
https://developers.google.com/speed/docs/best-practices/request#ServeFromCookielessDomain
Re-process the image using GD (or Imagick) and save the processed image. All others are just
funboring 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
if security is very important use database for save file name and renamed file name and here you can change extension of file to somthing like .myfile and make a php file for send image with headers . php can be more secure and you can use it in the img tag like blow :
also check file extension with EXIF before upload.
I will repeat something I posted in related question.
You may detect content type using Fileinfo functions (mime_content_type() in previous versions of PHP).
An excerpt form PHP manual on older Mimetype extension, which is now replaced by Fileinfo:
getimagesize()
may also do a good job, but most of the other checks you are performing are nonsense. For example why stringphp
is not allowed in filename. You are not going to include image file within PHP script, just because its name containsphp
string, are you?When it comes to re-creating images, in most cases it will improve security... until library you use is not vulnerable.
So which PHP extension suits best for secure image re-creation? I've checked CVE details website. I think the applicable trio are those extensions:
From the comparison I think GD suits best, because it has smallest number of security issues and they are quite old. Three of them are critical, but ImagMagick and Gmagick do not perform any better... ImageMagick seems to be very buggy (at least when it comes to security), so I choose Gmagick as the second option.
I'm using a php-upload-script that creates a new random 4-byte-number for each uploaded file, then XORs the content of the file with these 4 bytes (repeating them as often as necessary), and finally attaches the 4 bytes to the file before saving it.
For downloading, the 4 bytes have to be cut off of the file again, the contents will be XORed with them again and the result is sent to the client.
This way, I can be sure that the files I save on the server will not be executable or have any potential meaning whatsoever for any application. Plus I don't need any extra database to store filenames in.
Here is the code I'm using for this:
Upload:
Download: