[Security] Fix CodeQL alert #28: Uncontrolled data used in path expression#96
[Security] Fix CodeQL alert #28: Uncontrolled data used in path expression#96colin-d-fried wants to merge 1 commit into
Conversation
| file_path = os.path.join('/var/www/uploads/', filename) | ||
| base_dir = '/var/www/uploads/' | ||
| file_path = os.path.realpath(os.path.join(base_dir, filename)) | ||
| if not file_path.startswith(os.path.realpath(base_dir)): |
There was a problem hiding this comment.
🔴 Path traversal fix is bypassable due to missing trailing separator in startswith check
The startswith check on line 12 is vulnerable to a directory-prefix bypass. os.path.realpath('/var/www/uploads/') strips the trailing slash, returning /var/www/uploads. An attacker can supply filename=../uploads_evil/secret.txt, which resolves to /var/www/uploads_evil/secret.txt. This passes the startswith('/var/www/uploads') check because the string /var/www/uploads_evil/... does start with /var/www/uploads. The fix must append os.sep to the resolved base directory to ensure only true children of the base directory are matched.
| if not file_path.startswith(os.path.realpath(base_dir)): | |
| if not file_path.startswith(os.path.realpath(base_dir) + os.sep): |
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| file_path = os.path.join('/var/www/uploads/', filename) | ||
| base_dir = '/var/www/uploads/' | ||
| file_path = os.path.realpath(os.path.join(base_dir, filename)) | ||
| if not file_path.startswith(os.path.realpath(base_dir)): |
There was a problem hiding this comment.
Path traversal check bypassable via sibling directory names
High Severity
The startswith check is bypassable because os.path.realpath(base_dir) strips the trailing slash from /var/www/uploads/, producing /var/www/uploads. A sibling directory like /var/www/uploads_evil/ would also match startswith('/var/www/uploads'), allowing access to files outside the intended directory. The check needs to compare against the resolved base path with a trailing os.sep appended.


Summary
Fixes CodeQL alert #28: Uncontrolled data used in path expression
vulnerable_path_traversal.pyFix Applied
See the diff for the specific secure coding change applied.
Fixes #30
Note
Low Risk
Low-risk, localized security hardening in
download_filethat changes path resolution and may reject previously-accepted (but unsafe) inputs.Overview
Hardens the
/downloadendpoint invulnerable_path_traversal.pyagainst path traversal by resolving the requested filename withos.path.realpathand enforcing that the resulting path stays under the/var/www/uploads/base directory.Requests that resolve outside the allowed directory now return
403 Access deniedinstead of attempting tosend_filethe path.Written by Cursor Bugbot for commit cde6d93. This will update automatically on new commits. Configure here.