Security: Path Traversal in File Cleanup Command

[tomaioo]

Summary

Security: Path Traversal in File Cleanup Command

Problem

Severity: High | File: accounts/management/commands/clean_old_tmp_upload_files.py:L37

In accounts/management/commands/clean_old_tmp_upload_files.py, the code concatenates settings.FILE_UPLOAD_TEMP_DIR with f using simple string concatenation (settings.FILE_UPLOAD_TEMP_DIR + f). If settings.FILE_UPLOAD_TEMP_DIR does not end with a path separator, or if an attacker can influence the directory contents, this could lead to path traversal. More critically, the code iterates over all files in the directory and deletes them without proper validation, which could be exploited if the directory is not properly secured.

Solution

Use os.path.join() instead of string concatenation for path construction. Add validation to ensure files are within the expected directory using os.path.realpath() or similar. Consider using pathlib.Path for safer path handling.

Changes

• accounts/management/commands/clean_old_tmp_upload_files.py (modified)

[alastair]

Thanks for the report! We don't consider this a high severity issue, as it's an admin-only command and the configuration is set correctly so that it's not an issue in our environment.
Nevertheless it's a good idea to audit our use of string concatenation for path creation. I went ahead and opened #2138 to add a fix to this issue as well as some other issues that I found.
If you wish to contribute in the future then please take note of our PR template and issues system too.