php upload file problem

[flipper828]

Hello,

 

I have the following upload script that actually worked until I tried to add file type specifications.  I can't figure out what I did wrong.  I have been piecing things together so there is no telling what is wrong.  Help me please?

<?php
$fileName = $_FILES["file1"]["name"]; // The file name
$fileTmpLoc = $_FILES["file1"]["tmp_name"]; // File in the PHP tmp folder
$fileType = $_FILES["file1"]["type"]; // The type of file it is
$fileSize = $_FILES["file1"]["size"]; // File size in bytes
$fileErrorMsg = $_FILES["file1"]["error"]; // 0 for false... and 1 for true
if (!$fileTmpLoc) { // if file not chosen
    echo "ERROR: Please browse for a file before clicking the upload button.";
    exit();
}
if ($fileType=="image/png" || $fileType=="image/gif" || $fileType=="image/jpg") // checking file type
    echo "ERROR: Only file types .png, .gif or .jpg may be uploaded";
    exit();
}
if(move_uploaded_file($fileTmpLoc, "image_uploads/$fileName")){
    echo "$fileName upload is complete";
} else {
    echo "move_uploaded_file function failed";
}
?>

Thanks.

[requinix]

If the file is a PNG, or the file is a GIF, or the file is a JPEG, then show an error message that the image is supposed to be a PNG, GIF, or JPEG, and quit.

A bit backwards, yes?

 

I see you're grabbing the error code. Why aren't you checking it? You need to check that code to see what happened. Maybe the upload worked. Maybe the upload failed. Maybe the file was partially uploaded. You won't know until you check that error code.

 

While I'm at it, $fileType is not safe to use. The browser told you the type of file and for all you know the user told the browser to lie about that. You have to figure out the file type by yourself. For images you can use getimagesize.

 

The $fileName is not safe either. It can be absolutely anything the user wanted it to be and checking the file type, even by yourself, is not enough to be safe. For images specifically, unless they're too large you can load them into memory (so that PHP does a little bit of processing) and then save them out to the location you want with the name you want. Make sure it has the right extension too.

 

Speaking of names, you're not doing anything to check for overwriting files. Do you want that to happen?

[flipper828]

Thank you for analyzing for me.  

 

It looks like I have a lot more to learn.  

 

I'm off to research the things you mentioned.  

[Jacques1]

Trying to figure out the image type server-side doesn't help either, because a perfectly valid image can still contain malicious HTML (in a comment section, for example). Even worse, older versions of Internet Explorer will interpret the image as HTML, no matter what file extension and content type you serve.

 

Long story short: File uploads are a security nightmare. This isn't a quick job you can do in 10 minutes with a few lines of PHP code. If you absolutely must have this feature, take at least a week of your time to understand the risks and how to deal with them.

 

A rough overview:

• Store the files outside of your document root. This makes sure your webserver will never execute them as code, even if you screw up the file extension (which you did).
• The file name must be a random number to prevent people from overwriting existing files (accidentally or on purpose). Store the original file name in the database.
• There doesn't have to be a file extension, just store the type information in the database. If you do want an extension, then you must set it to one of the allowed values. Never accept the file extension from the user.
• Set the file permissions to the absolute minimum (e. g., 0600).
• Write an access script which takes an image ID, reads that image, looks up the type information in the database and then serves the image with the right type.
• Place the access script under a separate domain.

This gives you basic security, but it does not cover bad browsers turning your images into HTML. For that, you must prevent direct access to the images and instead embed them in an img element on a page.

 

[jazzman1]

I always use this wiki mozilla's overview and recommend you to read up on it when you're dealing with images - https://wiki.mozilla.org/WebAppSec/Secure_Coding_Guidelines#Uploads