0

My question is quite simple. Should I be as thorough in validation when I am not going to let users upload an image, but rather create an image from the source?

I was thinking that I will only use $_FILES['file']['tmp_name'] to create a new jpeg or png image with PHP functions.

On php.net I found this suggestion with the most votes, should I do it like this or is it overkill?

try {

    // Undefined | Multiple Files | $_FILES Corruption Attack
    // If this request falls under any of them, treat it invalid.
    if (
        !isset($_FILES['upfile']['error']) ||
        is_array($_FILES['upfile']['error'])
    ) {
        throw new RuntimeException('Invalid parameters.');
    }

    // Check $_FILES['upfile']['error'] value.
    switch ($_FILES['upfile']['error']) {
        case UPLOAD_ERR_OK:
            break;
        case UPLOAD_ERR_NO_FILE:
            throw new RuntimeException('No file sent.');
        case UPLOAD_ERR_INI_SIZE:
        case UPLOAD_ERR_FORM_SIZE:
            throw new RuntimeException('Exceeded filesize limit.');
        default:
            throw new RuntimeException('Unknown errors.');
    }

    // You should also check filesize here. 
    if ($_FILES['upfile']['size'] > 1000000) {
        throw new RuntimeException('Exceeded filesize limit.');
    }

    // DO NOT TRUST $_FILES['upfile']['mime'] VALUE !!
    // Check MIME Type by yourself.
    $finfo = new finfo(FILEINFO_MIME_TYPE);
    if (false === $ext = array_search(
        $finfo->file($_FILES['upfile']['tmp_name']),
        array(
            'jpg' => 'image/jpeg',
            'png' => 'image/png',
            'gif' => 'image/gif',
        ),
        true
    )) {
        throw new RuntimeException('Invalid file format.');
    }

    // You should name it uniquely.
    // DO NOT USE $_FILES['upfile']['name'] WITHOUT ANY VALIDATION !!
    // On this example, obtain safe unique name from its binary data.
    if (!move_uploaded_file(
        $_FILES['upfile']['tmp_name'],
        sprintf('./uploads/%s.%s',
            sha1_file($_FILES['upfile']['tmp_name']),
            $ext
        )
    )) {
        throw new RuntimeException('Failed to move uploaded file.');
    }

    echo 'File is uploaded successfully.';

} catch (RuntimeException $e) {
    echo $e->getMessage();
}
Alex
  • 5,671
  • 9
  • 41
  • 81

1 Answers1

0

I think you should do this(You be as thorough in validation).Reasons

  • What if someone uploads a harmful php file?

  • What if someone uploads a file with large size?

  • Other Security Problems

.
So thorough in validation is needed, else it will be a danger for site.The time needed for validation is less.So security is most important.

The code you got on php.net validates file size, extension etc which is perfect and minimizes the risk.

Also creating image from source needs more resources.So letting users upload image with thorough in validation is best. :)

Unni Babu
  • 1,839
  • 12
  • 16
  • Thanks for the answer. But it seems like an answer here disagree with you: http://stackoverflow.com/questions/15595592/php-validating-the-file-upload. "The only reliable method of image validation is to make a copy of it using GD or Imagick" So you should not let users to upload image to save resources. Or am I wrong? – Alex Jun 21 '15 at 20:42