Uploading a file to the server

Question

I wrote a simple PHP script which allows the user to upload a file to the server. It works, but I'm not very sure about security. Could you give me some hints about security mistakes?

In a nutshell: The user uploads a file from form.html, then a file is saved in uploads/ directory on the server, and a direct link to it is recorded in a database. Then that link is presented to the user, so he/she is able to download the file. The execution of that file is forbidden using .htaccess.

form.html

<div id="quickFileSubmit" style="margin-left: 20px; display: none">
    <table>
        <tr>
            <td>
                <form enctype="multipart/form-data" class="smallForm"
                      action="action.php?act=quickFileSubmit" method="post"
                      style="margin-top: 5px;">
                    <label style="font-size: 12px;">File:</label>
                    <input type="file" name="uploadfile" readonly="true" size="30"/>
                    <input type="submit" value="Submit" />
                </form>
            </td>
            <td style="width: 100%;"></td>
        </tr>
    </table>
</div>

action.php

//some actions

else if ($_REQUEST["act"] == "quickFileSubmit") {
    $basename = getUploadBasename();
    uploadFile();
    header("location:index.php");
}

function uploadFile() {

    if (strlen($_FILES['uploadfile']["name"]) > 0) {
        $target_path = "uploads/";
        $basename = basename( $_FILES['uploadfile']['name']);
        $target_path = $target_path . $basename;
        if (encode($basename) == $basename) {
            if(move_uploaded_file($_FILES['uploadfile']['tmp_name'], $target_path)) {

                // here goes saving of file info

                return $basename;
            } else{
                // upload failed.
                echo "File upload failed.";
                return null;
            }
        } else {
            echo "Illegal characters in file name.";
            return null;
        }
    }
}

Also, I have put .htaccess in the uploads dir:

.htaccess

SetHandler default-handler
Options -Indexes

Answer 1

One thing that immediately stands out to me is whether $_FILES['uploadfile'] actually exists for each upload request. Do an isset on it first to make sure a client doesn't send a post with fabricated input values - for example: some custom html form of their own making. If they do, you'll get 'function expects some value, boolean/null given' errors all over the place as is.

Or better yet, to allow client flexibility if they choose to use some sort of API for your website, just cycle through $_FILES array to work on whatever files they submit to this script. Your choice though if you want to instead maintain strict input, but it wouldn't matter really as long as your script receives the file input of a field named "uploadfile" from any source.

Also, for $_FILES[$x]["name"], validate or sanitize it to ensure it contains only alphanumeric, underscore, dash, and dot characters. Yes OS's can work with more characters than that, but not every protocol or application can (text, browsers or email editors breaking long filenames or links on spaces, etc).

Answer 2

It works, but I'm not very sure about security. Could you give me some hints about security mistakes?

This is not the correct approach to security, as an attacker can easily gain code execution:

• An attacker can upload a .htaccess file, overwriting your .htaccess file that is supposed to prevent execution, thus gaining code execution.
• SetHandler default-handler does not prevent code execution (at least not with my setup).
• Not all servers are configured to parse htaccess files (for performance and security reasons). PHP code should be secure under all - or at least all common - server configurations.

What you should do is check the file extension as well as the actual file type. The functions that are generally recommended for this are pathinfo and finfo_file respectively. Ideally, you should use whitelists, not blacklists.

It is a good idea to additionally forbid execution inside the upload directory, ideally by using server configuration as well as OS configuration, but this should not be your only line of defense.