Skip to content

[Security] Fix CodeQL alert #30: Uncontrolled data used in path expression#98

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

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

Conversation

@colin-d-fried
Copy link
Copy Markdown
Owner

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

Summary

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

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

Fix Applied

See the diff for the specific secure coding change applied.

Fixes #32


Note

Medium Risk
Introduces path canonicalization and a base-directory check for user-supplied document paths, which can affect file access behavior and potentially block previously reachable paths. Low code churn, but touches security-sensitive file I/O and relies on correct path validation.

Overview
Fixes a path traversal risk in view_document by resolving the requested doc path against a canonical /documents/ base directory and denying requests that escape that directory.

Replaces string path construction with os.path.realpath(os.path.join(...)) and returns 403 on validation failure before reading the file.

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


Open with Devin

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 2 additional findings in Devin Review.

Open in Devin Review

path = f"/documents/{doc}"
base_dir = os.path.realpath("/documents/")
path = os.path.realpath(os.path.join(base_dir, doc))
if not 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("/documents/") strips the trailing slash, yielding base_dir = "/documents". The check path.startswith(base_dir) on line 27 will therefore also match paths in sibling directories whose names share the same prefix (e.g., /documents_secret/). An attacker supplying doc=../documents_secret/passwd would produce path = "/documents_secret/passwd", which passes "/documents_secret/passwd".startswith("/documents")True, bypassing the security check and reading arbitrary files.

The fix is to ensure the resolved path starts with base_dir + os.sep (or equals base_dir exactly).

Suggested change
if not path.startswith(base_dir):
if not (path == base_dir or 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.

path = f"/documents/{doc}"
base_dir = os.path.realpath("/documents/")
path = os.path.realpath(os.path.join(base_dir, doc))
if not 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 bypassed via startswith without trailing separator

High Severity

The startswith check against base_dir is bypassable because os.path.realpath strips the trailing slash, leaving base_dir as "/documents". A sibling directory like "/documents_secret" would pass the path.startswith("/documents") check, allowing access to files outside the intended directory. The check needs to compare against base_dir + os.sep (or also allow path == base_dir) to be a correct path traversal guard.

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

1 participant