-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[WEB-4943]: add url has allowed host or scheme for validating valid redirections #7809
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,11 @@ | ||
| # Django imports | ||
| from django.utils.http import url_has_allowed_host_and_scheme | ||
| from django.conf import settings | ||
|
|
||
| # Python imports | ||
| from urllib.parse import urlparse | ||
|
|
||
|
|
||
| def _contains_suspicious_patterns(path: str) -> bool: | ||
| """ | ||
| Check for suspicious patterns that might indicate malicious intent. | ||
|
|
@@ -38,6 +43,21 @@ def _contains_suspicious_patterns(path: str) -> bool: | |
| return False | ||
|
|
||
|
|
||
| def get_allowed_hosts() -> list[str]: | ||
| """Get the allowed hosts from the settings.""" | ||
| base_origin = settings.WEB_URL or settings.APP_BASE_URL | ||
| allowed_hosts = [base_origin] | ||
| if settings.ADMIN_BASE_URL: | ||
| # Get only the host | ||
| host = urlparse(settings.ADMIN_BASE_URL).netloc | ||
| allowed_hosts.append(host) | ||
| if settings.SPACE_BASE_URL: | ||
| # Get only the host | ||
| host = urlparse(settings.SPACE_BASE_URL).netloc | ||
| allowed_hosts.append(host) | ||
|
Comment on lines
+48
to
+57
|
||
| return allowed_hosts | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Inconsistent Host Handling in URL ValidationThe |
||
|
|
||
|
|
||
| def validate_next_path(next_path: str) -> str: | ||
| """Validates that next_path is a safe relative path for redirection.""" | ||
| # Browsers interpret backslashes as forward slashes. Remove all backslashes. | ||
|
|
@@ -92,7 +112,14 @@ def get_safe_redirect_url(base_url: str, next_path: str = "", params: dict = {}) | |
| base_url = base_url.rstrip('/') | ||
| if params: | ||
| encoded_params = urlencode(params) | ||
| return f"{base_url}/?next_path={validated_path}&{encoded_params}" | ||
| url = f"{base_url}/?next_path={validated_path}&{encoded_params}" | ||
| else: | ||
| url = f"{base_url}/?next_path={validated_path}" | ||
|
|
||
| return f"{base_url}/?next_path={validated_path}" | ||
| # Check if the URL is allowed | ||
| if url_has_allowed_host_and_scheme(url, allowed_hosts=get_allowed_hosts()): | ||
| return url | ||
|
|
||
| # Return the base URL if the URL is not allowed | ||
| return base_url | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing import for
get_allowed_hostsfunction. The function is used on line 204 but not imported, which will cause a NameError at runtime.