Skip to content

Python: Fix URL redirection validation sanitizers - #22872

Open
owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:fix/python-url-redirect-netloc
Open

owen-mc wants to merge 1 commit into
github:mainfrom
owen-mc:fix/python-url-redirect-netloc

Conversation

@owen-mc

@owen-mc owen-mc commented Oct 22, 2026

Copy link
Copy Markdown
Contributor

Addresses #22846.

@github-actions

Copy link
Copy Markdown
Contributor

QHelp previews:

python/ql/src/Security/CWE-601/UrlRedirect.qhelp

URL redirection from remote source

Directly incorporating user input into a URL redirect request without validating the input can facilitate phishing attacks. In these attacks, unsuspecting users can be redirected to a malicious site that looks very similar to the real site they intend to visit, but which is controlled by the attacker.

Recommendation

To guard against untrusted URL redirection, it is advisable to avoid putting user input directly into a redirect URL. Instead, maintain a list of authorized redirects on the server; then choose from that list based on the user input provided.

If this is not possible, then the user input should be validated in some other way, using a maintained framework helper that accounts for how browsers interpret URLs.

Example

The following example shows an HTTP request parameter being used directly in a URL redirect without validating the input, which facilitates phishing attacks:

from flask import Flask, request, redirect

app = Flask(__name__)

@app.route('/')
def hello():
    target = request.args.get('target', '')
    return redirect(target, code=302)

If you know the set of valid redirect targets, you can maintain a list of them on the server and check that the user input is in that list:

from flask import Flask, request, redirect

VALID_REDIRECT = "http://cwe.mitre.org/data/definitions/601.html"

app = Flask(__name__)

@app.route('/')
def hello():
    target = request.args.get('target', '')
    if target == VALID_REDIRECT:
        return redirect(target, code=302)
    else:
        # ignore the target and redirect to the home page
        return redirect('/', code=302)

If an allowlist is not practical, use a maintained framework helper to validate the redirect target. For Django applications, url_has_allowed_host_and_scheme checks whether a URL is safe to redirect to:

from django.http import HttpResponseRedirect
from django.shortcuts import redirect
from django.utils.http import url_has_allowed_host_and_scheme
from django.views import View

class RedirectView(View):
    def get(self, request, *args, **kwargs):
        target = request.GET.get('target', '')
        if url_has_allowed_host_and_scheme(target, allowed_hosts=None):
            return HttpResponseRedirect(target)
        else:
            # ignore the target and redirect to the home page
            return redirect('/')

Note that url_has_allowed_host_and_scheme handles backslashes correctly, so no additional processing is required.

Do not rely on urlparse or urlsplit alone to validate redirect targets. These functions do not interpret all URLs in the same way as browsers:

  • Many browsers accept backslash characters (\) as equivalent to forward slash characters (/) in URLs, but these parsers do not.
  • Mistyped URLs such as https:/example.com or https:///example.com are parsed as having an empty netloc attribute, while browsers will still redirect to the correct site.
  • URLs such as ///example.com and ////example.com are parsed as having both an empty netloc and an empty scheme, but browsers interpret them as URLs with a host name.
    Hand-written validation must also account for whitespace and control characters that browsers may ignore. Validate the same normalized value that is used in the redirect.

References

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

The security-sensitive sanitizer redesign still has alert-suppression gaps and precision tradeoffs that need maintainer review.

4 open issues
What changed in this PR

Addresses #22846 by tightening Python’s py/url-redirection sanitizers and correcting guidance for validating redirect targets.

Changes:

  • Adds urlsplit support and stricter checks for potentially empty network locations.
  • Expands regression cases and expected alerts for incomplete validation.
  • Recommends maintained framework helpers, removes the unsafe example, and adds change notes.
File Description
python/​ql/​test/​query-tests/​Security/​CWE-601-UrlRedirect/​UrlRedirect.expected Updates expected alerts and flow paths.
python/​ql/​test/​query-tests/​Security/​CWE-601-UrlRedirect/​test.py Adds validation and allowlist regression cases.
python/​ql/​src/​Security/​CWE-601/​UrlRedirect.qhelp Explains parser differences and recommends framework helpers.
python/​ql/​src/​Security/​CWE-601/​examples/​redirect_good2.py Removes the unsafe manual-validation example.
python/​ql/​src/​change-notes/​2026-10-22-url-redirect-validation.md Documents query and help changes.
python/​ql/​lib/​semmle/​python/​frameworks/​Stdlib/​Urllib.qll Extends parser support and tightens sanitizer guards.
python/​ql/​lib/​change-notes/​2026-10-22-url-redirect-validation.md Documents sanitizer and allowlist restrictions.

Review effort: Max

exists(DataFlow::AttrRead read |
read = DataFlow::exprNode(e) and
read.getAttributeName() = "hostname" and
read.getObject().getALocalSource() = getUrlParseCall()
Comment on lines +128 to +131
forex(DataFlow::LocalSourceNode source | source = DataFlow::exprNode(e).getALocalSource() |
exists(Expr collection |
collection = source.asExpr() and
(collection instanceof List or collection instanceof Tuple or collection instanceof Set) and
prefix = call.getArg(0).asExpr().(Tuple).getAnElt()
) and
prefix.(StringLiteral).getText() in ["/", "//", "///"] and
not exists(call.getArg(1)) and
Comment on lines +131 to +140
(collection instanceof List or collection instanceof Tuple or collection instanceof Set) and
forall(Expr elt |
elt =
[
collection.(List).getAnElt(), collection.(Tuple).getAnElt(),
collection.(Set).getAnElt()
]
|
excludesEmptyString(elt)
)

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants