Skip to content

[Security] Fix CodeQL alert #29: Uncontrolled data used in path expression#97

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

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

Conversation

@colin-d-fried
Copy link
Copy Markdown
Owner

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

Summary

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

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

Fix Applied

See the diff for the specific secure coding change applied.

Fixes #31


Open with Devin

Note

Medium Risk
Adds path canonicalization and base-directory enforcement for user-supplied filenames; low code churn but touches file-system access where mistakes could still allow bypasses or block legitimate files.

Overview
Hardens the /read endpoint in vulnerable_path_traversal.py by resolving the requested filename against a fixed base directory and denying requests whose real path escapes that directory.

This replaces direct open(file_name) with realpath-based validation and reads from the validated safe_path, returning 403 on traversal attempts.

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

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

with open(file_name, 'r') as f:
base_dir = os.path.realpath('/var/www/files/')
safe_path = os.path.realpath(os.path.join(base_dir, file_name))
if not safe_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 bypass via startswith check without trailing separator

os.path.realpath('/var/www/files/') strips the trailing slash, so base_dir is /var/www/files. The check safe_path.startswith(base_dir) can be bypassed by accessing sibling directories whose names share the same prefix (e.g., /var/www/files_secret/). For example, filename=../files_secret/passwd resolves to /var/www/files_secret/passwd, which passes the startswith('/var/www/files') check, allowing reads outside the intended directory.

Example bypass

GET /read?filename=../files_secret/sensitive.txt

  • base_dir = /var/www/files
  • safe_path = os.path.realpath('/var/www/files/../files_secret/sensitive.txt') = /var/www/files_secret/sensitive.txt
  • '/var/www/files_secret/sensitive.txt'.startswith('/var/www/files')True → access granted
Suggested change
if not safe_path.startswith(base_dir):
if not safe_path.startswith(base_dir + os.sep):
Open in Devin Review

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

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.

with open(file_name, 'r') as f:
base_dir = os.path.realpath('/var/www/files/')
safe_path = os.path.realpath(os.path.join(base_dir, file_name))
if not safe_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 fix bypassable via sibling directory prefix

High Severity

os.path.realpath strips the trailing slash from '/var/www/files/', so base_dir becomes '/var/www/files'. The startswith check then incorrectly permits access to sibling directories with a common prefix (e.g., /var/www/files_secret/), because '/var/www/files_secret/foo'.startswith('/var/www/files') is True. The check needs to compare against base_dir + os.sep to ensure the resolved path is actually within the intended directory.

Fix in Cursor Fix in Web

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 #29] Uncontrolled data used in path expression

1 participant