Skip to content

fix Incomplete URL substring sanitization Unvalidated Redirects and BXSS #9435

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: develop
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Apr 12, 2025

'amazonaws.com' in git_parameters['host'] or

fix the problem need to parse the URL and check the host value correctly. Instead of checking if "amazonaws.com" is a substring of the host, we should use the urlparse function to extract the hostname and then check if it ends with "amazonaws.com". This ensures that the check is accurate and not prone to bypasses.

  1. Import the urlparse function from the urllib.parse module.
  2. Parse the URL to extract the hostname.
  3. Check if the hostname ends with "amazonaws.com".

Sanitizing untrusted URLs is a common technique for preventing attacks such as request forgeries and malicious redirections. Usually, this is done by checking that the host of a URL is in a set of allowed hosts. However, treating the URL as a string and checking if one of the allowed hosts is a substring of the URL is very prone to errors. Malicious URLs can bypass such security checks by embedding one of the allowed hosts in an unexpected location. Even if the substring check is not used in a security-critical context, the incomplete check may still cause undesirable behaviors when the check succeeds accidentally.

POC

The following code checks that a URL redirection will reach the evil-redacted.com domain.

from flask import Flask, request, redirect
from urllib.parse import urlparse

app = Flask(__name__)

# Not safe, as "evil-example.net/evil.com" would be accepted

@app.route('/some/path/bad1')
def unsafe1(request):
    target = request.args.get('target', '')
    if "example.com" in target:
        return redirect(target)

# Not safe, as "benign-looking-prefix-evil.com" would be accepted

@app.route('/some/path/bad2')
def unsafe2(request):
    target = request.args.get('target', '')
    if target.endswith("example.com"):
        return redirect(target)



#Simplest and safest approach is to use an allowlist

@app.route('/some/path/good1')
def safe1(request):
    allowlist = [
        "example.com/home",
        "example.com/login",
    ]
    target = request.args.get('target', '')
    if target in allowlist:
        return redirect(target)

#More complex example allowing sub-domains.

@app.route('/some/path/good2')
def safe2(request):
    target = request.args.get('target', '')
    host = urlparse(target).hostname
    #Note the '.' preceding example.com
    if host and host.endswith(".example.com"):
        return redirect(target)

The first two examples show unsafe checks that are easily bypassed. In unsafe1 the attacker can simply add evil.com anywhere in the url, for the vulnerable http://<aws-host>/evil.com. In safe2, urlparse is used to parse the URL, then the hostname is checked to make sure it ends with <aws-host>.evil.com.

References

SSRF
XSS Unvalidated Redirects and Forwards Cheat Sheet
CWE-20

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.

1 participant