File Upload

It’s very common that applications will, at some point or another, need to allow users to upload a file (either for use or just for storage) somewhere within the application. While it seems simple enough, how this function is implemented can be pretty critical due to the potential risks associated with how file uploads are handled. 

Take a look at this quick example, just to give a better visual understanding of what we mean. 

Let’s say this is an application that allows users to upload a profile picture:

public string UploadProfilePicture(FormFile uploadedFile)
{
    // Generate path to save the uploaded file at
    var path = $"./uploads/avatars/{request.User.Id}/{uploadedFile.FileName}";  

    // Save the file
    var localFile = File.OpenWrite(path);
    localFile.Write(uploadedFile.ReadToEnd());
    localFile.Flush();
    localFile.Close();

    // Update the profile picture
    UserProfile.UpdateUserProfilePicture(request.User, path)

    return path;
}

This would be a very basic upload function that also happens to be vulnerable to Path Traversal. 

Depending on the exact implementation of the application, an attacker could upload another page/script (think .asp, .aspx, or .php files) that would make it possible to call directly and execute arbitrary code. This could also allow the overriding of existing files. 

Issue 1 - Saving to local disk rather than external data store

As the use of cloud services becomes more commonplace, applications are delivered in containers, high-availability setups have become standard and the practice of writing uploaded files to the local disk of the application should essentially be avoided at all costs. 

Files should be uploaded to a form of central storage where possible (Block storage, or database). This can avoid entire classes of security vulnerabilities in this case. 

Issue 2 - Not validating extensions 

In many cases where a file upload vulnerability is exploited, it relies on the ability to upload a file with a specific extension. As such, it's highly advisable to make use of an ‘allow-list’ of extensions for files that can be uploaded. 

Make sure to use the methods provided by your language/framework to get the extension of the file to avoid issues, like null-byte injection. 

It may also be tempting to validate the content type of the upload, but doing so can make it very brittle, given that content types used for specific files can differ between operating systems. It also doesn’t actually tell you anything about the file itself since the content type is purely a mapping from an extension. 

Issue 3 - Not preventing path traversal

Another common issue with file uploads is that they also tend to be vulnerable to path traversal. That’s a whole thing on its own, so rather than trying to summarise it here, give the full guideline on Path Traversal a look.

More examples

Below, we’ve got a few more examples of both secure and insecure file uploads you can take a look at. 

C# - Insecure

public string UploadProfilePicture(IFormFile uploadedFile)
{
    // Generate path to save the uploaded file at
    var path = $"./uploads/avatars/{request.User.Id}/{uploadedFile.FileName}";

    // Save the file
    var localFile = File.OpenWrite(path);
    localFile.Write(uploadedFile.ReadToEnd());
    localFile.Flush();
    localFile.Close();

    // Update the profile picture
    UserProfile.UpdateUserProfilePicture(request.User, path)

    return path;
}

C# - Secure

public List<string> AllowedExtensions = new() { ".png", ".jpg", ".gif"};

public string UploadProfilePicture(IFormFile uploadedFile)
{
    // NOTE: The best option is to avoid saving files to the local disk.
    var basePath = Path.GetFullPath("./uploads/avatars/");

    // Prevent path traversal by not utilizing the provided file name. Also needed to avoid filename conflicts.
    var newFileName = GenerateFileName(uploadedFile.FileName);

    // Generate path to save the uploaded file at
    var canonicalPath = Path.Combine(basePath, newFileName);

    // Ensure that we did not accidentally save to a folder outside of the base folder
    if(!canonicalPath.StartsWith(basePath))
    {
        return BadRequest("Attempted to save file outside of upload folder");
    }

    // Ensure only allowed extensions are saved
    if(!IsFileAllowedExtension(uploadedAllowedExtensions))
    {
        return BadRequest("Extension is not allowed");
    }

    // Save the file
    var localFile = File.OpenWrite(canonicalPath);
    localFile.Write(uploadedFile.ReadToEnd());
    localFile.Flush();
    localFile.Close();

    // Update the profile picture
    UserProfile.UpdateUserProfilePicture(request.User, canonicalPath)

    return path;

public bool GenerateFileName(string originalFileName) {
    return $"{Guid.NewGuid()}{Path.GetExtension(originalFileName)}";
}

public bool IsFileAllowedExtension(string fileName, List<string> extensions) {
    return extensions.Contains(Path.GetExtension(fileName));
}

‍

Java - Insecure

@Controller
public class FileUploadController {

   @RequestMapping(value = "/files/upload", method = RequestMethod.POST)
   @ResponseBody
   public ResponseEntity<String> uploadFile(@RequestParam("file") MultipartFile file, @AuthenticationPrincipal User user) {

       try {

           String uploadPath = "./uploads/avatars/" + principal.getName() + "/" + file.getOriginalFilename();

           File transferFile = new File(uploadPath);
           file.transferTo(transferFile);

       } catch (Exception e) {
           return new ResponseEntity<>("Upload error", HttpStatus.INTERNAL_SERVER_ERROR);
       }

       return new ResponseEntity<>(uploadPath, HttpStatus.CREATED);
   }
}

Java - Secure

@Controller
public class FileUploadController {

    @RequestMapping(value = "/files/upload", method = RequestMethod.POST)
    @ResponseBody
    public ResponseEntity<String> uploadFile(@RequestParam("file") MultipartFile file, @AuthenticationPrincipal User user) {

        try {
            String baseFolder = Paths.get("./uploads/avatars/").normalize();
            String uploadPath = Paths.get(baseFolder.toString() +
GenerateFileName(file.getOriginalFilename())).normalize();
           // Make sure that the extension is an allowed type
            if(!IsAllowedExtension(file.getOriginalFilename()) {
                return new ResponseEntity<>("Extension not allowed", HttpStatus.FORBIDDEN);
            }

            // Make sure that the file is not saved outside of the upload root
           if(!uploadPath.toString().startsWith(baseFolder.toString()))            {
                return new ResponseEntity<>("Files are not allowed to be saved outside of the base folder.", HttpStatus.FORBIDDEN);
           }

            File transferFile = new File(uploadPath.toString());
            file.transferTo(uploadPath.toString());

        } catch (Exception e) {
            return new ResponseEntity<>("Upload error", HttpStatus.INTERNAL_SERVER_ERROR);
        }

        return new ResponseEntity<>(uploadPath, HttpStatus.CREATED);
    }

    private string GenerateFileName(String fileName) {
        return UUID.randomUUID().toString() + "." + FilenameUtils.getExtension(fileName);
    }

    private boolean IsAllowedExtension(String fileName) {
        String[] allowedExtensions = {"jpg", "png", "gif"};
        String extension = FilenameUtils.getExtension(filename);
        return allowedExtensions.contains(extension);
    }
}

Python - Flask - Insecure

@app.route('/files/upload', methods=['POST'])
def upload_file():

    file = request.files['file']

    savedFilePath = os.path.join("./uploads/avatars/", file.filename)
    file.save(savedFilePath)

    return savedFilePath

Python - Flask - Secure

@app.route('/files/upload', methods=['POST'])
def upload_file():

    file = request.files['file']
    baseFolder = os.path.normpath("./uploads/avatars/")
    savedFilePath = os.path.normpath(os.path.join(baseFolder, generate_file_name(file.filename)))

    # Ensure that the extension is in the allowed set
    if not is_extension_allowed(file.filename):
        return "This extension is not allowed"

    # Ensure that the file we're trying to save to is not outside the base
    if not savedFilePath.startsWith(baseFolder):
        return "Attempted to save file outside of base folder"

    file.save(savedFilePath)

    return savedFilePath

def generate_file_name(filename):
    return str(uuid.uuid4()) + os.path.splitext(filename)[1]

def is_extension_allowed(filename):
    return os.path.splitext(filename)[1] in (".png", ".jpg", ".gif")