HOW To write a secure picture upload script

Good Day,
Please, I need help on HOW To write a secure picture upload script…below is the code am working with which I believe more secure measure is still needed…pls kindly help me with idea or code to come up with better way to secure file picture upload.


if (preg_match('/^image\/p?jpeg$/i', $_FILES['pic']['type']) or
preg_match('/^image\/gif$/i', $_FILES['pic']['type']) or
preg_match('/^image\/jpg$/i', $_FILES['pic']['type']) or
preg_match('/^image\/(x-)?png$/i', $_FILES['pic']['type']))
{

$pictureName = time(). rand(0,999999999) . $_FILES['pic']['name'];

$tpFile = htmlspecialchars($_FILES['pic']['tmp_name'], ENT_QUOTES, 'UTF-8');
$pictureName = htmlspecialchars($pictureName, ENT_QUOTES, 'UTF-8');

}
else {
$updErr = 'Please upload a JPEG, GIF, or PNG image file';
}

if (!is_uploaded_file($_FILES['pic']['tmp_name']) )
{
$updErr = 'Please upload a JPEG, GIF, or PNG image file';
}else {move_uploaded_file($tpFile, "profilePhoto/".$pictureName);}

All help is welcome.

Generally, it is better to use finfo_file than invoking regex though you’d need to ensure the fileinfo extension is enabled from your php.ini. Here is a sample function using file_info:

/**
@param $file Should be a $_FILE instance
@param $legalTypes an array of allowed MIME types
function uploadFile ($file, array $legalTypes) {
    $tmpName = $file['tmp_name'];
    $name = $file['name'];
    $size = $file['size'];
    $error = $file['error'];

    if($error !== UPLOAD_ERR_OK){
        //we could not upload the file, you can inspect $error for more info
        return "Failed to upload file";
    }
    $finfo = finfo_open(FILEINFO_MIME_TYPE);
    $mime = finfo_file($finfo, $tmpName);
    if(!in_array($mime, $legalTypes)){
        //The file mime type is not in our allowed types.
        return "Uploaded file is not valid";
    }
    $pictureName = time(). rand(0,999999999) . $name;
    $pictureName = htmlspecialchars($pictureName, ENT_QUOTES, 'UTF-8');
    move_uploaded_file($tmpName, "profilePhoto/".$pictureName);
}

And then you can use it like so:

$imageTypes = [
    "image/jpeg", "image/jpg", "image/png", "image/gif"
];
uploadFile($_FILES['pic'], $imageTypes);

Also, you would want to consider setting/checking for a size limit on uploaded files.

1 Like

I would personally recommend you use something simple like this!

Instead of writing your own like I have done before you are better of using a class as they are more efficent and more easily extendable!

Or you can us this one as a challenge for youself and add the mime type because that is currently what is missing from this class!
https://github.com/kazzkiq/ImageUploader

Or if you want to stick with the function() here is one I wrote a while back but it has a few errors which i’m sure you could figure out.

<?php
/**
 * @param $tag			| the name of the input field that was used to upload the file
 * @param $size			| the allowed size of the file in bytes
 * @param $format		| the allowed formats for the file
 * @param $type			| the allowed file mime types for the file (extra security, never trust the user)
 * @param $filename		| the base name of the file after upload
 * @param $directory	| the directory in which the file will be stored
 * @param $checkimg		| true for only image / false for other files 
 * @param $checkdim		| if true will check uploaded image dimentions with $w & $h
 * @param $w		    | if not null sets the maximum width of uploaded images
 * @param $h		    | if not null sets the maximum height of uploaded images
 * @param $error		| the error generated from the function
 * @return boolean      | returns true if the file was uploaded successfully, otherwise false
 *
 * Simple Upload File Function
 */
function uploadFile($tag, $size, $format, $type, $filename, $directory, $checkimg, $checkdim, $w=null, $h=null, &$error=null)
{
	// Added Image Security (Enable/Disable)
	if($checkimg) {
		if(getimagesize($_FILES[$tag]["tmp_name"]) === false) {
			$error = 'Upload a valid image file';
			return;
		}
	}
	
	// If Image Enabled  - Check Width & Height
	if($checkimg && $checkdim) {
		list($width, $height) = getimagesize($_FILES[$tag]["tmp_name"]);
		if($width > $w || $height > $h) {
			die("This image does not meet the image dimension limits.");
		}
	}
	
	// Check if File Exists
	if(file_exists($target_file)) {
		$error = 'File already exists';
		return;
	}
	
	// Check File Size
	if($_FILES[$tag]["size"] > $size) {
		$error = 'Your file is larger than '.($size/1024).' KBs';
		return;
	}
	
	// Allow Certain File Extentions
	@$imageFileType = array_pop(explode(".", strtolower($_FILES[$tag]["name"])));
	$target_file =  $directory.'/'.$filename.'.'.$imageFileType;

	if(!in_array($imageFileType, $format)) {
		$error = 'The allowed formats are '.implode(', ', $format);
		return;
	}

	// Allow Certain File Mime Types
	if (!in_array($_FILES[$tag]["type"], $type)) {
		$error = 'The allowed mime formats are '.implode(', ', $type);
		return;
	}

	// If Validation Passed Try Upload
	if (move_uploaded_file($_FILES[$tag]["tmp_name"], $target_file)) {
		return true;
	}

	$error = 'There was an error uploading your file';
	return false;
}

and then for example usage:

					if(uploadFile('fileUpload', 10000000, array('jpeg','jpg','png','gif','zip'), array('image/jpeg','image/png','image/gif','application/zip', 'application/octet-stream', 'application/x-zip-compressed', 'multipart/x-zip'), array_pop(array_reverse(explode(".", $formData['filename']))), 'uploads/suploads/', false, false, $error) === TRUE) {
						if($this->support->insertFile($formData)) {
							flash('message', 'Your file was uploaded!', 'alert alert-success mb-0');
							redirect('user/ticket/'.$id);
						} else {
							die(LANG['something-went-wrong']);
						}
					} else {
						echo $error;
					}

Note: This does have two errors…

**Notice** : Only variables should be passed by reference in **C:\xampp\htdocs\site\app\controllers\Help.php** on line **247**

**Notice** : Undefined variable: error in **C:\xampp\htdocs\site\app\controllers\Help.php** on line **247**

But good luck!

As a general rule for security, you should not trust anything that it sent by a user. So for your script $_FILES['name']['mime'] is a user supplied value. Normally a browser determines the mime type and sends it to your server, but there is nothing stopping a malicious user from supplying a value that doesn’t correspond with what they are uploading. For example a malicious PHP script, but the add a mime type image/gif so your script will accept it. Then you have a malicious php script on your site, which can be called from the browser. When that happens all bets are off.

So you should indeed do as was said above, and check the file for yourself. The best method you can use is check mime type of the file, and then make sure that the file has the file extension that goes with that mime type. So for example for the malicious PHP script, if you detect a mime type of image/gif 1, but the file has an extension of .php, rename it so the extension becomes .php.gif. That way, when the user tries to open that file in the browser, it will not be handled by PHP because of gif extension, thus the hacker no longer has a way to execute their malicious script.

1 This is possible. Mime type detectors look at a few fixed spots in the file, and if there are bytes there that they expect they will conclude it is that mime type. So you could create a php file that will be detected as image/gif.

2 Likes

Thanks everyone for taking your time to help me out…Am still trying to be better using PHP.

@idoko

@jack55

@rpkamp

I have try to work out the script you post here but I come up with the code below…Please kindly help me look into the code and help me fix what is missing with the problem am facing below…Am trying to resize the image when user upload…but facing some problem…

Problem facing now:

1: If big file is uploaded with correct mime type is will display the error message File Too Big and still move the file into the folder /profilePage/bgImage/. If the file is big I do not want it to move the file to the folder, just display the error message only.

2: When mime type that is not allow it upload it will display the error message Invalid File Type and still move the file into the folder /profilePage/bgImage/. If the file is not allow I do not want it to move the file to the folder, just display the error message only.

3: if a file like .pm3 is upload which is not allow…error message Invalid File upload display and I also get php warning error message finfo_file(): Empty filename or path in
fileinfo is enable

4: When everything is correct…allow mime type and file size is okay the code will still move (double image) into the folder /profilePage/bgImage/ …If everything is fine only move the file with ( min- )

///////////////////////

//error check
$errorStatus = false;

function compressImage($source_url, $destination_url, $quality) {

		$info = getimagesize($source_url);

    		if ($info['mime'] == 'image/jpeg')
          $image = imagecreatefromjpeg($source_url);

            elseif ($info['mime'] == 'image/jpg')
          $image = imagecreatefromjpeg($source_url);

          elseif ($info['mime'] == 'image/gif')
          $image = imagecreatefromgif($source_url);

          elseif ($info['mime'] == 'image/png')
          $image = imagecreatefrompng($source_url);

        if (isset($image)) {

           imagejpeg($image, $destination_url, $quality);
        return $destination_url;  }
    		
	}



//////////////////////////


$legalTypes = ["image/jpeg", "image/jpg", "image/png", "image/gif"];

$gbImgName = time(). rand(0,999999999) . $_FILES['bgImg']['name'];
$gbImgNameVal = strtolower(htmlspecialchars($gbImgName, ENT_QUOTES, 'UTF-8'));

$tmpFile = htmlspecialchars($_FILES['bgImg']['tmp_name'], ENT_QUOTES, 'UTF-8');

//check mime type
$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mime = finfo_file($finfo, $tmpFile);

if(!in_array($mime, $legalTypes)){
//The file mime type is not in our allowed types.
$uploadErr  = "Invalid File Type";
$errorStatus = true; }

$maxsize    = 2050000;
$size = htmlspecialchars($_FILES['bgImg']['size'], ENT_QUOTES, 'UTF-8');
if(($size >= $maxsize) || ($size == 0)) {
$uploadErr  = "File too Big";
$errorStatus = true; }

if (!is_uploaded_file($tmpFile) )
{
$uploadErr = 'Invalid File upload';
$errorStatus = true;
}
else{
move_uploaded_file($tmpFile,"../../profilePage/bgImage/".$gbImgNameVal);
$source_image = "../../profilePage/bgImage/".$gbImgNameVal;
$image_destination = "../../profilePage/bgImage/"."min-".$gbImgNameVal;
$compress_imagesVal = compressImage($source_image, $image_destination, 70);
}


//Good to go......
if(!$errorStatus) {


//set image.....


}// $errorStatus

Ahhh Code OCD going off :joy:

Please can you show the function in use like I did above in my below as your code is very very messy!

uploadFile('fileUpload', 10000000, array('jpeg','jpg','png','gif','zip'), array('image/jpeg','image/png','image/gif','application/zip', 'application/octet-stream', 'application/x-zip-compressed', 'multipart/x-zip'), array_pop(array_reverse(explode(".", $formData['filename']))), 'uploads/suploads/', false, false, $error) === TRUE

Thanks

@jack55

the only function I use is

compressImage($source_image, $image_destination, 70);

Which I use to compress the image size

and

$legalTypes = ["image/jpeg", "image/jpg", "image/png", "image/gif"];

to hold the only allow mime type

Ok what PHP version are you using?

And try this piece of code:

// Allow Certain File Extentions
	#@$imageFileType = array_pop(explode(".", strtolower($_FILES['bgImg']["name"])));

	// Allow Certain File Mime Types
	if (!in_array($_FILES['bgImg']["type"], $legalTypes)) {
		$error = 'The allowed mime formats are '.implode(', ', $legalTypes);
		return;
	}

That code will check your array, compare, if not in will display error messages with alllowed mime types

Try that instead

@jack55

I try the code…same issue still… Am using PHP 7

I do not want the file to move to the folder when error is detect…I want the script to stop and display error message only and not to move the file to the folder…

I would personally then use this source code and then customise. It’s PHP 7+
https://github.com/kazzkiq/ImageUploader

It’s nice, simple and easy to customise. All it needs is the mime type and your compressing

Also, it’s tackling 2 concerns (file upload and image resize) instead of one, isn’t tested, and only support ImageMagick (and not GD). For image uploading I would recommend the method I’ve describe earlier - it’s not that hard to code that, and for image manipulation I would suggest imagine.

Repeat after me: never trust user input.

The PHP manual also warns for this value:

The mime type of the file, if the browser provided this information. An example would be “image/gif” . This mime type is however not checked on the PHP side and therefore don’t take its value for granted.

https://secure.php.net/manual/en/features.file-upload.post-method.php

Honestly, PHP would be a lot better off if it did not provide this value in the array

1 Like

I would recommend you listen to @rpkamp. He is much more of a pro at PHP than I am! Still a student over here :joy:

Thanks Scallio for the pointers! This one would be much better then.

If you must use a package for uploading, I’d rather use https://github.com/brandonsavage/Upload, and then as I said before, imagine for processing.

The advantage is that that upload package has tests, which makes it much more likely that it works correctly and keeps on working correctly.

1 Like

I was able to control the script to allow only image file and I was able to detect image.php.gif file as bad invalid file type with finfo_open(FILEINFO_MIME_TYPE)…below is the code…


$legalTypes = ["image/jpeg", "image/jpg", "image/png", "image/gif"];

$tmpFile = htmlspecialchars($_FILES['bgImg']['tmp_name'], ENT_QUOTES, 'UTF-8');

//check mime type
$finfo = finfo_open(FILEINFO_MIME_TYPE);
$mime = finfo_file($finfo, $tmpFile);
finfo_close($finfo);


if(!in_array($mime, $legalTypes)){
//The file mime type is not in our allowed types.
$uploadErr  = Invalid File Type!.; }

When a file like image.php.gif with some php code detected it will display the error message Invalid File Type and the file is not uploaded…

Some thought are welcome.

That’s not what I meant :slight_smile:

What I meant is something like this:

$allowedTypes = [
  // mime type => expected extension
  'image/jpeg' => 'jpg'
  'image/jpg' => 'jpg',
  'image/png' => 'png',
  'image/gif' => 'gif'
];

// no need for htmlentities here
$tmpFile = $_FILES['bgImg']['tmp_name'];

//check mime type
$mimeFinfo = finfo_open(FILEINFO_MIME_TYPE);
$mime = finfo_file($mimeFinfo, $tmpFile);
finfo_close($mimeFinfo);

if (!array_key_exists($mime, $legalTypes, true)) {
  //The file mime type is not in our allowed types.
  $uploadErr = 'Invalid File Type!';
}

// See what extension the file should have
$expectedExtension = $allowedTypes[$mime];

// See what the actual extension of the file is
$extFinfo = finfo_open(PATHINFO_EXTENSION);
$actualExtension = strtolower(finfo_file($extFinfo, $tmpFile));
finfo_close($extFinfo);

$fileName = $_FILES['bgImg']['name'];

if ($actualExtension !== $expectedExtension) {
  // ffile extension doesn't correspond with mime type
  // add expected extension to filename to prevent
  // exploits

  $fileName = sprintf('%s.%s', $fileName, $expectedExtension);
}

// now you can safely use $fileName here
3 Likes

Thanks once again…The script is running fine but I notice

1: when expected file type is upload the filename get another added extension. (Good the image display)

  1. when a file like image.php.gif with some php code or badfile.php or textfile.txt is upload the script will submit but the file do not move to the folder which is fine but is that how is going to be working?

And I want to know if all upload file will always get expected extension added to it?

Thanks once again…you really helping me.

In the array here you want to use $allowedTypes variable right?

if (!array_key_exists($mime, $legalTypes, true)) {
  //The file mime type is not in our allowed types.
  $uploadErr = 'Invalid File Type!';
}

That might still need some tweaking. To be honest I just gave the gist of the code, I didn’t fully test it

Everything regarding moving and saving the file should be supplied by you as I have no idea how you’d want to go about that.

Only if the current extension is different from the expected extension.

Yes

1 Like

Thanks for your time @rpkamp . I have been working on this script to make it better but am facing some issue with file size now… I only want to allow image file size that is 2mb but when image file that is more that 2mb is uploaded the script will display the error message in below array…the file type is fine but the size is larger than 2mb…when file that is less than 2mb is upload everything work fine.


if (!array_key_exists($mime, $allowedTypes, true)) {
  //The file mime type is not in our allowed types.
  $uploadErr = 'Invalid File Type!. Please upload a JPEG, GIF, or PNG image';
}

BUT ignore the code line that check for the size…

$maxsize    = 2000000;
$size = $_FILES['bgImg']['size'];
if(($size >= $maxsize) || ($size == 0)) {
$uploadErr  = "Image File too Big!";}

Pls I will be very glad if you can help me make the code work fine with size check

Seems fine. Could you post the entire script please?