File Uploads and Downloads when files are stored outside of the webroot? (PHP)

Question

I'm building a web app that is essentially a job board -- users create "jobs/projects" which are then presented as a global "to-do" list for the rest of the company to see.

One of the features that I'm trying to implement is an "attachments" feature that would allow users to upload files as part of each project's data.

The idea is to allow users to upload files securely, then allow other users to download the attachments.

For example, if we were creating product packaging for a client, then it would be nice to be able to attach the client's logo (a .pdf or whatever) as part of the project's data, so that any designer who views the project using our job board can download that file.

---

Using a combination of common upload techniques and referencing a PHP book (PHP and MySQL for Dynamic Websites - by Larry Ullman), I built the following PHP script:

[..]
// check if the uploads form has been submitted:
if($_SERVER['REQUEST_METHOD'] == 'POST') {

//check if the $_FILES global has been set:
if (isset($_FILES['upload'])) {

    //create a function to rewrite the $_FILES global (for readability):
    function reArrayFiles($file) {
        $file_ary = array();
        $file_count = count(array_filter($file['name']));
        $file_keys = array_keys($file);
        for ($i=0; $i<$file_count; $i++) {
            foreach ($file_keys as $key) {
                $file_ary[$i][$key] = $file[$key][$i];
            }
        }
        return $file_ary;
    }

    //create a variable to contain the returned data & call the function 
        //**Quick note: I thought simply stating 'reArrayFiles($_FILES['upload']);' would be enough, but I guess not
    $file_ary = reArrayFiles($_FILES['upload']);

    //establish an array of allowed MIME file types for the uploads:
    $allowed = array(
        'image/pjpeg', //.jpeg
        'image/jpeg',
        'image/JPG',
        'image/X-PNG', //.png
        'image/PNG',
        'image/png',
        'image/x-png',
        'image/gif', //.gif
        'application/pdf', //.pdf
        'application/msword', //.doc
        'application/vnd.openxmlformats-officedocument.wordprocessingml.document', //.docx
        'application/vnd.ms-excel', //.xls
        'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet', //.xlsx
        'text/csv', //.csv
        'text/plain', //.txt
        'text/rtf', //.rtf
    );

    //these are two arrays for containing statements and errors that occured for each individual file upload
    //so I can choose exactly where these "errors" print on page, rather than printing where the script as a whole is called
    $statement = array();
    $upload_error = array();

    //multi-file upload, so perform checks and actions on EACH file upload individually:
    foreach ($file_ary as $upload) {

        //validate the uploaded file's MIME type using finfo:
        $fileinfo = finfo_open(FILEINFO_MIME_TYPE); //open handle

        //read the file's MIME type (using magic btyes), then check if it is w/i the allowed file types array
        if ( in_array((finfo_file($fileinfo, $upload['tmp_name'])), $allowed) ) {

            //check the file's MIME type AGAIN, but this time using the rewritten $_FILES['type'] global
            //it may be redundant to check the file type twice, but I felt this was necessary because some files made it past the first conditional
            if ( in_array($upload['type'], $allowed) && ($upload['size'] < 26214400) ) {

                //set desired file structure to store files:
                //the tmp directory is one level outside my webroot
                //the '$job_data[0]' variable/value is the unique job_id of each project
                //here, it is used to create a folder for each project's uploads -- in order to keep them organized
                $structure = "../tmp/uploads/job_" . $job_data[0] . "/";

                //check if the folder exists:
                if (file_exists($structure) && is_dir($structure)) {

                    //if directory already exists, get file count: (files only - no directories or subdirectories)
                    $i = 0;
                    if (($handle = opendir($structure))) {
                        while (($file = readdir($handle)) !== false) {
                            if (!in_array($file, array('.','..')) && !is_dir($structure.$file)) 
                                $i++;
                        }
                        closedir($handle);
                        $file_count = $i;
                    }
                } else {
                    //directory does not exist, so create it
                    //files are NOT counted b/c new directories shouldn't have any files) -- '$file_count == 0'
                    mkdir($structure);
                }

                //if file count is less than 10, allow file upload:
                //this limits the project so it can only have a maximum of 10 attachments
                if ($file_count < 10) {

                    if (move_uploaded_file($upload['tmp_name'], "$structure{$upload['name']}")) {
                        $statement[] = '<p>The file has been uploaded!</p>';
                    } else {
                        $statement[] = '<p class="error">The file could not be transfered from its temporary location -- Possible file upload attack!</p>';
                    }

                } else if ($file_count >= 10) { 

                    //if there are already 10 or more attachments, DO NOT upload files, return statement/error
                    $statement[] = '<p class="error">Only 10 attachments are allowed per Project.</p>'; 
                } 

            //ELSE FOR 2ND FILE TYPE CHECK:
            } else {
                $statement[] = '<p class="error">Invalid basic file type.</p>';
            }

            //set an error msg to $upload_error array if rewritten $_FILES['error'] global is not 0
            //this section of code omitted; literally every upload script does this
            if ($upload['error'] > 0) {
                switch ($upload['error']) {
                    [...]
                }
            }

            //remove the temp file if it still exists after the move/upload
            if ( file_exists($upload['tmp_name']) && is_file($upload['tmp_name']) ) {
                unlink ($upload['tmp_name']);
            }

        //ELSE FOR 1ST FILE TYPE CHECK
        } else {
            $statement[] = '<p class="error">Invalid MIME file type.</p>';
        }

        //close the finfo module
        finfo_close($fileinfo);

    } //END OF FOREACH

} //END OF isset($_FILES['upload']) conditional

}//END OF $_SERVER['REQUEST_METHOD'] == 'POST' conditional

---

My HTML looks like this:

<form enctype="multipart/form-data" action="edit-job.php" method="post">
<input type="hidden" name="MAX_FILE_SIZE" value="26214400"/>
<fieldset>
    <legend>Upload Project Files</legend>
    <input type="file" name="upload[]"/>
    <input type="file" name="upload[]"/>
    <input type="file" name="upload[]"/>
    <input type="file" name="upload[]"/>
    <input type="file" name="upload[]"/>
    <p>Max Upload Size = 25MB</p>
    <p><b>Supported file types:</b> .jpeg, .png, .gif, .pdf, .doc, .docx, .xls, .xlsx, .csv, .txt, .rtf</p>
</fieldset>
<input type="submit" name="submit" value="Edit Job"/>
</form>

---

To summarize, I've presented a multiple file upload PHP script (with validation) and accompanying HTML.

My method DOES NOT use a MySQL database where a table equates project IDs with associated attachments/files, as I've seen other upload methods use.

It simply creates unique folders for each project's attachments in a common location outside the webroot, because that's supposed to be more secure.

At this point, I admit that this all seems rather unorthodox, but it was working well up until I had to worry about user downloads!

---

Regardless, here are my questions:

(1) How do I allow users to download files using my structure (outside the webroot)?

I initially tried creating basic links to the files, like so:

<a href="../tmp/uploads/{unique_folder}/{file_name}" target="_blank">{file_name}</a>';

But that obviously didn't work due to restrictions/inherent security. Then, I found out that it was better to use a separate "download.php" file (correct me if I am wrong), like so:

'<a href="download.php?id=' . $job_data[0] . '&file_name=' . $file . '" target="_blank">' . $file . '</a>';

(passing variables to a separate .php file)

But what should that .php file contain? I've read all sorts of things about php's header() function, recreating .pdf files from the tmp originals, etc.

I just can't make sense of it all...

Here a link to what I'm talking about:

http://web-development-blog.com/archives/php-download-file-script - it sounds like you use php to access the files rather than allow a user's browser to do it; can anyone verify this resource?

(2) Am I doing anything wrong?

• Should I be using a MySQL database for the attachments (like I mentioned)?
• Are there obvious security flaws in my upload script?
• Should there be a loading bar mechanism for slow uploads? Etc.

I am worried about the integrity of my web app as a whole; I don't want to be affected by SQL injection/ other hacking methods.

But further than that, I want to weed out any bad practices I may have as a novice developer.

---

Your feedback is very much appreciated; let me know if you need any additional info.

Answer 1

Concerning (1):

Theory is: You store the file somewhere outside the webroot to prevent direct access and to prevent execution on the server side. You have to be able to find it, when the user provides the correct parameters. (However, you should be careful, how do the files get presented to the user? A database could be helpful here) If security is a concern, you have to make sure that a user cannot access files that he should not have access to (when he has the right download link but doesn't have permissions for example, because so far your download links don't seem to be very cryptic).

The script you mentioned in your question is quite alright, although I'd probably just use fpassthru instead of the feof-fread-echo-loop. The idea here is essentially to figure out the mime type, add that to the headers and then dump the contents into the output stream.

Concerning (2):

Using a database especially with prepared statements is quite safe and provides some extra possibilities. (Like adding some comment to the attachment, a timestamp, filesize, reordering, ...)

You don't check the upload_name, this could very well be ../../your webroot/index.php or something similar. My advice would be to store the files that are uploaded as something unimaginative as "file_ID" and store that id with the original filename in the database. You should probably also remove any leading multiple dots, slashes ("directory") and similar.

Loading bars ... well that's taste I guess.