Skip to content

[Security] Fix CodeQL alert #31: Uncontrolled data used in path expression#99

Open
colin-d-fried wants to merge 1 commit into
mainfrom
security/codeql-31-path-traversal-image-fix
Open

[Security] Fix CodeQL alert #31: Uncontrolled data used in path expression#99
colin-d-fried wants to merge 1 commit into
mainfrom
security/codeql-31-path-traversal-image-fix

Conversation

@colin-d-fried
Copy link
Copy Markdown
Owner

@colin-d-fried colin-d-fried commented Mar 26, 2026

Summary

Fixes CodeQL alert #31: Uncontrolled data used in path expression

Field Value
Severity high
File vulnerable_path_traversal.py
CWE CWE-022
Alert CodeQL Alert #31

Fix Applied

See the diff for the specific secure coding change applied.

Fixes #33


Note

Low Risk
Low-risk, localized change that only affects how /image resolves and validates file paths; potential risk is blocking previously-accepted (but unsafe) paths.

Overview
Hardens the /image route against path traversal by resolving the requested filename against a real ./static/images/ base directory and denying requests whose resolved path escapes that base.

Replaces string path concatenation with os.path.join + os.path.realpath and returns 403 on invalid paths before calling send_file.

Written by Cursor Bugbot for commit a6b7c17. This will update automatically on new commits. Configure here.

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

img_path = "./static/images/" + img_name
base_dir = os.path.realpath("./static/images/")
img_path = os.path.realpath(os.path.join(base_dir, img_name))
if not img_path.startswith(base_dir):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Path traversal check bypassable via sibling directory prefix

High Severity

The startswith check against base_dir can be bypassed because os.path.realpath strips trailing slashes. If base_dir resolves to e.g. /app/static/images, a request for a file in a sibling directory like /app/static/images_evil/secret.txt would pass the img_path.startswith(base_dir) check. The comparison needs to ensure the path is within the directory by appending os.sep to base_dir (or also allowing an exact match with base_dir itself).

Fix in Cursor Fix in Web

Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

Comment on lines +51 to +52
if not img_path.startswith(base_dir):
return 'Access denied', 403
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Path traversal bypass via startswith without trailing separator

The startswith check on line 51 is bypassable. os.path.realpath strips trailing slashes, so base_dir will be e.g. /abs/path/to/static/images. An attacker can request ?name=../images_evil/secret.txt, which resolves to /abs/path/to/static/images_evil/secret.txt. This path passes img_path.startswith(base_dir) because the string /abs/path/to/static/images_evil/... starts with /abs/path/to/static/images, allowing access to files outside the intended directory. The check should use img_path.startswith(base_dir + os.sep) (or also allow an exact match with base_dir).

Suggested change
if not img_path.startswith(base_dir):
return 'Access denied', 403
if not (img_path == base_dir or img_path.startswith(base_dir + os.sep)):
return 'Access denied', 403
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[CodeQL #31] Uncontrolled data used in path expression

1 participant