File uploads in Laravel - Single and multiple

Question

I have been reading up on file uploads in Laravel, reading through the documentation and looking at best practices. In my application, I have two instances: an upload area for single files, and an area for uploading multiple files.

The singular one allows users to enter their own file name whereas the multi file upload does not.

The method for uploading a single file

/**
 * Store a newly created resource in storage.
 *
 * @param  \Illuminate\Http\Request  $request
 * @return \Illuminate\Http\Response
 */
public function store(Request $request)
{
    // Get every extension that we allowed
    $all_ext = implode(',', $this->allExtensions());

    $this->validate($request, [
            'name'=>'required|unique:file_meta_data|min:3|max:40',
            'file' => 'required|file|mimes:' . $all_ext . '|max:2048'
        ]);

    // Grab data from the request to fill the necessary fields
    $name = $request->get('name');
    $department = $request->get('department');
    $category = $request->get('category');
    $uploadedFile = $request->file('file');

    // Get the extension and then use this to work out the file type
    $file->size = $uploadedFile->getSize();
    $extension = $uploadedFile->getClientOriginalExtension();
    $type = $this->getType($extension);

    // Create a new instancce of this model
    $file = new FileMetaData();

    $file->name = $name;
    $file->department = $department;
    $file->category = $category;
    $file->type = $type;
    $file->extension = $extension;

    // Upload all the given files to Storage, within a specific directory
    if ($department != '') {
        $path = $file->storeAs('library/' . $department, $name);
        $category = null;
    } elseif ($category != '') {
        $path = $file->storeAs('library/' . $category, $name);
        $department = null;
    }

    // Grab the filepath so we can store it in the database
    $file->filepath = $path;

    // Finally, check that the file exists and save the model
    if (Storage::exists($path)) {
        $file->save();

        return redirect('editable/templates-and-tools')->with('success', 'File has been added');
    }
}

As you can see this takes various attributes and then places the file within storage, in the specified directory.

The multiple uploading method

/**
 * Process uploading of multiple files
 *
 * @param Request $request
 * @return void
 */
public function bulkUpload(Request $request)
{
    // Get every extension that we allowed
    $all_ext = implode(',', $this->allExtensions());

    // Initialize a file upload counter
    $fileCount = 0;

    // Get the category and department from the form
    $department = $request->get('department');
    $category = $request->get('category');

    // Loop through each file and add it to storage
    foreach ($request->file('file') as $file) {
        // Get the meta data for each file
        $name = $file->getClientOriginalName();
        $extension = $file->getClientOriginalExtension();
        $type = $this->getType($extension);
        $size = $file->getSize();

        // Upload all the given files to Storage, within a specific directory
        if ($department != '') {
            $path = $file->storeAs('library/' . $department, $name);
            $category = null;
        } elseif ($category != '') {
            $path = $file->storeAs('library/' . $category, $name);
            $department = null;
        }

        // Grab the filepath so we can store it in the database
        $file->filepath = $path;

        // Create the database entries for the newly uploaded files
        FileMetaData::UpdateOrCreate([
            'name' => $name,
            'department' => $department,
            'category' => $category,
            'type' => $type,
            'extension' => $extension,
            'size' => $size,
            'filepath' => $path
        ]);

        $fileCount++;
    }

    return redirect('editable/templates-and-tools')->with('success', $fileCount . ' files have been added');
}

I'm trying to improve these two methods so that they're cleaner and have better overall security. Also, when using the multiple file upload, you can only rename the files by editing the database entry once they're uploaded, via a seperate form.

How could I improve these methods?

Perhaps I should also use a better UI element like a Jquery file uploader?

Answer 1

What will happen if another file with the same name already exists? This may show interesting error messages that can lead to information disclosure.

Here some suggestions,

• It is recommended to use an algorithm to determine the filenames. For instance, a filename can be a MD5 hash of the name of file plus the date of the day.
• Limit the file size to a maximum value in order to prevent denial of service attacks (on file space or other web application’s functions such as the image resizer).
• Prevent from overwriting a file in case of having the same hash for both.
• Ensure that files with double extensions (e.g. "file.php.txt") cannot be executed especially in Apache.
• All the control characters and Unicode ones should be removed from the filenames and their extensions without any exception. Also, the special characters such as ";", ":", ">", "<", "/" ,"\", additional ".", "*", "%", "$", and so on should be discarded as well. If it is applicable and there is no need to have Unicode characters, it is highly recommended to only accept Alpha-Numeric characters and only 1 dot as an input for the file name and the extension; in which the file name and also the extension should not be empty at all (regular expression: [a-zA-Z0-9]{1,200}.[a-zA-Z0-9]{1,10}).

For additional info, owasp