Skip to content

fix: Incomplete URL substring sanitization Unvalidated Redirects and Forwards #736

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Feb 27, 2025

📝 Description

patched(fix) for #735

To fix the problem, we should parse the URL and check the hostname to ensure it matches the expected domain. This can be done using the urlparse function from the urllib.parse module. We will extract the hostname from the URL and check if it ends with the expected domain.

  • Replace the substring check "-example.com" in result with a more robust check that parses the URL and verifies the hostname.
  • Update the test to use the urlparse function to extract the hostname and check if it ends with the expected domain.

@odaysec odaysec requested a review from a team as a code owner February 27, 2025 11:14
@odaysec odaysec requested review from jriddle-linode and ykim-akamai and removed request for a team February 27, 2025 11:14
assert "-example.com" in result
from urllib.parse import urlparse
parsed_url = urlparse(result)
assert parsed_url.hostname and parsed_url.hostname.endswith("-example.com")

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
-example.com
may be at an arbitrary position in the sanitized URL.
@ykim-akamai
Copy link
Contributor

Thank you for updating the test case. Getting a minor assertion failure from make test-int TEST_CASE=test_list_slave_domain

AssertionError: assert (None)
E        +  where None = ParseResult(scheme='', netloc='', path='32190231741125163985209000-example.comslaveactive32190051741124805550044547exa...mple.clone-1740764427076302488-inttestsdk.orgmasteractivepathiel-test123@linode.com', params='', query='', fragment='').hostname

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.

2 participants