HOW To write a secure picture upload script

PHP
21

Thanks so much @rpkamp , Here is the script…

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

if(strlen($_FILES['bgImg']['name']) > 65)
{
    $uploadErr = 'Image name too long!.';
}

if($_FILES['bgImg']['error'] !== UPLOAD_ERR_OK){
        
        $uploadErr = "Failed to upload file";
        }

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


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


$tmpFile = $_FILES['bgImg']['tmp_name'];

if(!is_uploaded_file($tmpFile) ) {
$uploadErr = 'Invalid File upload!.';
}

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

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';
}

// 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'];
$fileName = time(). rand(999,99999) . $fileName;
$fileName = strtolower(htmlspecialchars($fileName, ENT_QUOTES, 'UTF-8'));

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

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

Thanks for been there…

22

I don’t see an issue there :thinking:

Also, 2MB = 2 * 1024 * 1024 bytes, not 2000000 :slight_smile:

23

Is the problem that the script only deals with one error message, so the only one seen is the last one the script finds? So any check after the filesize would overwrite the message.

24

@droopsnoot pls how can I check for the error, can you pls help me with the code. Thanks

25

Thanks @rpkamp, i really want to catch the image size if it bigger than 2MB… I change to * 1024 * 1024 bytes but same issue…

26

I’m not sure what you mean. You’re already checking for errors, but each time you find a new error, you overwrite the value of $uploadErr with the latest error message, so if you have more than one error, you only report the last one you find.

One way would be to exit as soon as you’ve found the first error, though this can be annoying for users. Another would be to append each error onto the error string

$uploadErr .= "latest error message";

Yet another way would be to build up an array of error messages. Your choice, really.

27

oh…programming is really interesting…I can now catch the error when bigger file is upload… I move the line of code that check for the file size down…

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

if(strlen($_FILES['bgImg']['name']) > 65)
{
    $uploadErr = 'Image name too long!.';
}

if($_FILES['bgImg']['error'] !== UPLOAD_ERR_OK){
        
        $uploadErr = "Failed to upload file";
        }

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


$tmpFile = $_FILES['bgImg']['tmp_name'];

if(!is_uploaded_file($tmpFile) ) {
$uploadErr = 'Invalid File upload!.';
}

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

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';
}

// 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'];
$fileName = time(). rand(999,99999) . $fileName;
$fileName = strtolower(htmlspecialchars($fileName, ENT_QUOTES, 'UTF-8'));

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

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


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

I hope it can stay safe that way? and again I get PHP warning message that say’s…

finfo_file(): Empty filename or path in

when empty how can I catch that?

28

Don’t call finfo_file() if the filename or path is empty? Check first, throw it as another error.

29

@droopsnoot how pls…help me with sample code pls.

30

I usually use an array for form validation errors. First make an empty array:-

$uploadErrs = array();

Then in stead of overwriting a variable every time there is another error, you just add the error to the array:-

if(strlen($_FILES['bgImg']['name']) > 65)
{
    $uploadErrs[] = 'Image name too long!.';
}

if($_FILES['bgImg']['error'] !== UPLOAD_ERR_OK){
        
        $uploadErrs[] = "Failed to upload file";
        }
// Etc...

Then at the appropriate point in the script, check if there is anything in that array:-

if(count($uploadErrs)){
        // There are errors, return to form and display errors to user.
}
else{
      // All went well, continue to process the upload...
}

Then to display the errors you can render a list using a foreach loop.

1 Like
31

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.