Skip to content

Potential fix for code scanning alert no. 636: URL redirection from remote source #7760

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sriramveeraghanta merged 1 commit into from
Sep 11, 2025
Merged

Potential fix for code scanning alert no. 636: URL redirection from remote source #7760

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions apps/api/plane/utils/path_validator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,20 @@


def validate_next_path(next_path: str) -> str:
"""Validates that next_path is a valid path and extracts only the path component."""
"""Validates that next_path is a safe relative path for redirection."""
# Browsers interpret backslashes as forward slashes. Remove all backslashes.
next_path = next_path.replace("\\", "")
Comment on lines +7 to +8
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

Removing backslashes entirely may be too aggressive and could break legitimate paths that contain escaped characters. Consider replacing backslashes with forward slashes instead: next_path = next_path.replace('\\', '/') to maintain path structure while preventing bypass attempts.

Suggested change
# Browsers interpret backslashes as forward slashes. Remove all backslashes.
next_path = next_path.replace("\\", "")
# Browsers interpret backslashes as forward slashes. Replace backslashes with forward slashes.
next_path = next_path.replace("\\", "/")

Copilot uses AI. Check for mistakes.
parsed_url = urlparse(next_path)

# Ensure next_path is not an absolute URL
# Block absolute URLs or anything with scheme/netloc
if parsed_url.scheme or parsed_url.netloc:
next_path = parsed_url.path # Extract only the path component
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

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

The current logic extracts the path component from absolute URLs instead of rejecting them entirely. This could still allow malicious redirects if the extracted path contains dangerous sequences. Consider returning an empty string immediately when scheme or netloc is detected: return ''.

Suggested change
next_path = parsed_url.path # Extract only the path component
return ""

Copilot uses AI. Check for mistakes.
Comment on lines +11 to 13
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Block protocol-relative URLs ('//...') and multi-slash variants explicitly.

Cases like '////evil.com/path' may be parsed as a path but are treated by browsers as network-path references. Guard them.

     if parsed_url.scheme or parsed_url.netloc:
         next_path = parsed_url.path  # Extract only the path component
+
+    # Disallow protocol-relative/network-path references
+    if next_path.startswith("//"):
+        return ""
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Block absolute URLs or anything with scheme/netloc
if parsed_url.scheme or parsed_url.netloc:
next_path = parsed_url.path # Extract only the path component
# Block absolute URLs or anything with scheme/netloc
if parsed_url.scheme or parsed_url.netloc:
next_path = parsed_url.path # Extract only the path component
# Disallow protocol-relative/network-path references
if next_path.startswith("//"):
return ""
🤖 Prompt for AI Agents 
In apps/api/plane/utils/path_validator.py around lines 11 to 13, the current
check only blocks absolute URLs via scheme/netloc but misses protocol-relative
or multi-leading-slash inputs like '//evil.com' or '////evil.com/path' which
browsers treat as network-path references; update the validator to explicitly
detect and reject any input whose original string or parsed_url.path begins with
two or more slashes (e.g. startswith('//') or re.match(r'^/{2,}', ...)) and
treat them as invalid before accepting parsed_url.path so these network-path
references cannot slip through.


# Ensure it starts with a forward slash (indicating a valid relative path)
if not next_path.startswith("/"):
# Must start with a forward slash and not be empty
if not next_path or not next_path.startswith("/"):
return ""

# Ensure it does not contain dangerous path traversal sequences
# Prevent path traversal
if ".." in next_path:
return ""
Comment on lines +19 to 21
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 10, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Catch encoded traversal ('%2e%2e') by decoding once before checking.

Naively searching for '..' misses percent-encoded variants. Decode once for detection; still return the original sanitized path.

-    # Prevent path traversal
-    if ".." in next_path:
+    # Prevent path traversal (including percent-encoded variants)
+    if ".." in unquote(next_path):
         return ""

Add import (outside this hunk):

-from urllib.parse import urlparse
+from urllib.parse import urlparse, unquote
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Prevent path traversal
if ".." in next_path:
return ""
++ b/apps/api/plane/utils/path_validator.py
@@ imports
from urllib.parse import urlparse, unquote
@@ def validate_next_path(...):
- # Prevent path traversal
# Prevent path traversal (including percent-encoded variants)
if ".." in unquote(next_path):
return ""
🤖 Prompt for AI Agents 
In apps/api/plane/utils/path_validator.py around lines 19 to 21, the traversal
check only looks for literal ".." and misses percent-encoded variants; decode
the incoming path once (e.g., using urllib.parse.unquote) into a local variable
and check that decoded string for ".." (and other traversal patterns if
desired), but continue to return the original sanitized path as before; also add
the required import (from urllib.parse import unquote) at the top of the file
outside this hunk.


Expand Down
Loading