FileUpload Base64 extension fix

[hblankenship]

[sc-5645]

Previously, when adding Files to a test, the API does not validate whether the title supplied is associated with a valid file extension. This fix validates the title of the 'files' object for a supported extension, and then return an error message if the file extension is not supported or does not exist.

This fix applies to the generic json parser and the finding/files, test/files and engagement/files endpoints.

[dryrunsecurity]

DryRun Security Summary

The pull request includes various improvements and enhancements to the Dojo application, focusing on serializers, JSON parser, and file upload functionality, with a strong emphasis on security practices such as file extension validation and careful handling of security-sensitive operations.

Expand for full summary

Summary:

The code changes in this pull request cover various improvements and enhancements to the Dojo application, with a focus on the serializers, JSON parser, and file upload functionality. The key changes include:

1. Serializers: The FileSerializer class in the api_v2/serializers.py file has been updated to delegate the file extension validation to the FileUpload model's clean() method, which is an improvement in terms of code organization and maintainability.

2. JSON Parser: The GenericJSONParser class in the tools/generic/json_parser.py file has been enhanced to handle file uploads and improve the endpoint data handling. These changes introduce security-sensitive operations that require careful validation and sanitization to prevent potential vulnerabilities.

3. File Upload Model: The FileUpload model in the models.py file has been updated with a new clean() method that validates the file extension against a list of valid extensions. This is a good security practice to prevent the upload of potentially malicious files.

4. Test Case: A new JSON file, test_with_image_no_ext.json, has been added to the unittests/scans/generic/ directory. This test case introduces a finding with an image file that has no file extension, which could potentially be a security issue if the application is not properly handling files without extensions.

Overall, these code changes appear to be focused on improving the functionality and security of the Dojo application, particularly in the areas of file upload handling and JSON data parsing. As an application security engineer, I would recommend thoroughly reviewing the implementation of these features to ensure that they are secure and do not introduce any vulnerabilities.

Files Changed:

1. dojo/api_v2/serializers.py: The changes in this file delegate the file extension validation to the FileUpload model's clean() method, which is an improvement in terms of code organization and maintainability.

2. dojo/tools/generic/json_parser.py: The changes in this file enhance the GenericJSONParser class to handle file uploads and improve the endpoint data handling. These changes require careful review to ensure the security of the file upload and endpoint data processing.

3. dojo/models.py: The changes in this file introduce a new clean() method in the FileUpload model that validates the file extension against a list of valid extensions. This is a good security practice to prevent the upload of potentially malicious files.

4. unittests/scans/generic/test_with_image_no_ext.json: This new file introduces a test case with a finding that includes an image file without an extension, which could potentially be a security issue if the application is not properly handling files without extensions.

Code Analysis

We ran 9 analyzers against 4 files and 0 analyzers had findings. 9 analyzers had no findings.

Riskiness

🟢 Risk threshold not exceeded.

View PR in the DryRun Dashboard.

[github-actions]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[github-actions]

Conflicts have been resolved. A maintainer will review the pull request shortly.

[mtesauro]

Approved

[mtesauro]

Interesting and a good catch. I'm not sure I would have thought of this case