feat: judge open-redirect flows on the destination a browser resolves

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Precision
+
+- `open-redirect` judges a flow on the destination a browser resolves from the target's constant text, with its own criteria rather than SSRF's. A reference on the current site keeps the browser there, so the flow is refuted. Such a reference is `/` followed by a character that is not a slash, a backslash, a space or a control character, as in `"/profile/" + id`; or `?`, `#`, or a relative path whose first segment holds no `:`. A fixed absolute host makes the flow a hotspot, since the input chooses the path, where that host may redirect again. Everything else stays a vulnerability, including `"/" + next`: `//evil.com` is another host. The URL facts both rules read are one shared proof, `coretrace_python.taint.urls`, and `ssrf` now reads it too, with the same verdicts. The regression corpus is unchanged: its three `open-redirect` findings redirect to values built without constant text (#168).
+
 ## 0.14.0 (2026-09-26)
 
 ### Precision

docs/plugins.md

@@ -117,7 +117,9 @@ that must not leave the application through a log, a third party or plain storag
 
 A rule may know more about its own sinks than the refutation does: `judge(ctx, function,
 flow, verdict)` returns the rule's verdict on a flow, given the refutation's, and by
-default returns it unchanged. `ssrf` reads there how the URL is built. The refutation's
+default returns it unchanged. `ssrf` and `open-redirect` read there what the URL's
+constant text proves, a proof they share (`coretrace_python.taint.urls`) and each
+judges by its own criteria. The refutation's
 verdict, which other consumers of the flow such as the advisory correlation read, does
 not change; a rule reading more analyses declares them in its `requires`.
 

docs/usage.md

@@ -117,7 +117,7 @@ variable is not treated that way yet.
 | `ssrf` | The URL of an HTTP client request (Requests, httpx, `urllib`), by position or as `url=`; the body, headers and query parameters are not the destination. How `+` and f-strings build the URL can prove part of its destination. When constant text fixes `scheme://host` and ends the authority with `/`, `?` or `#` before any input (`"https://api.example.com/users/" + id`, or a module-level string bound once and never rebound), the flow is a hotspot: the input still chooses the path, which may reach a sensitive resource, and a redirect may lead elsewhere. With the path fixed too, the input reaching the query or fragment only, and the call disabling redirects (`allow_redirects=False`, `follow_redirects=False`), the destination is fixed and the flow is refuted. Anything unproven stays a vulnerability: a base of unknown value (imported, a call result), a constant leaving the authority open (`"https:/"`, no `/` after the host, a variable port), `urljoin`, `%`, `format`, a URL built in a helper function. |
 | `xss` | An HTTP response body built without escaping. A Django template rendered by `render_to_string` escapes, where the analyzer can establish it (below). |
 | `insecure-deserialization` | `pickle`, `marshal`, `dill`, `jsonpickle`, `shelve`, and the YAML loaders (`yaml.load`, `load_all`, `full_load`, `unsafe_load` and their several-document versions); `yaml.load` and `load_all` given a safe loader (`SafeLoader`, `BaseLoader`) are not. |
-| `open-redirect` | An HTTP redirect target. |
+| `open-redirect` | An HTTP redirect target, judged on the destination a browser resolves from the target's constant text, as `+` and f-strings build it. A reference on the current site keeps the browser there, where any further redirect is the project's own code, analysed on its own, so the flow is refuted. Such a reference is `/` followed by a character that is not a slash, a backslash, a space or a control character (`"/profile/" + id`), `?`, `#`, or a relative path whose first segment ends in the text without a `:`. A fixed absolute host (`"https://example.com/" + path`) sends the browser there, but the input chooses the path, where that host may redirect again: a hotspot. Anything else stays a vulnerability. That covers `"/" + next`, since `//evil.com` is another host, a host left open, a base of unknown value and a target built in a helper function. |
 | `plaintext-credential-storage` | A parameter named like a password stored in a database without hashing. Medium confidence, since a name is a hint. |
 | `exploitable-vulnerability` | An API affected by an advisory of a vulnerable requirement, see Dependencies. |
 

src/coretrace_python/bundled/security/open_redirect/open_redirect.py

@@ -1,12 +1,26 @@
-"""Open redirect: attacker-controlled input reaching a open redirect sink."""
+"""Open redirect: attacker-controlled input reaching a open redirect sink.
+
+The rule draws its verdict from what the target's constant text proves (``taint.urls``),
+as a browser resolves it. A reference on the current site (``/profile/``, ``?page=``,
+``profile/``) keeps the browser on the site, where any further redirect is the project's
+own code, analysed on its own: refuted. A fixed absolute host sends the browser there,
+but the input chooses the path, where that host may redirect again: a hotspot. Anything
+unproven, ``"/" + next`` included, stays a vulnerability.
+"""
 
 from __future__ import annotations
 
 from typing import ClassVar
 
+from coretrace_python.abstract.strings import ModuleStringsAnalysis
+from coretrace_python.analysis import AnyAnalysis
 from coretrace_python.findings import Severity
-from coretrace_python.plugins import TaintDetector
-from coretrace_python.taint import TaintKind
+from coretrace_python.findings.refutation import Status, Verdict
+from coretrace_python.hir import nodes
+from coretrace_python.ir.ssa import SSAAnalysis
+from coretrace_python.plugins import PluginContext, TaintDetector
+from coretrace_python.taint import TaintFlow, TaintKind
+from coretrace_python.taint.urls import flow_url
 
 
 class OpenRedirectPlugin(TaintDetector):
@@ -15,3 +29,19 @@ class OpenRedirectPlugin(TaintDetector):
     kind: ClassVar[TaintKind] = TaintKind.REDIRECT
     severity: ClassVar[Severity] = Severity.MEDIUM
     title: ClassVar[str] = "Open redirect"
+    requires: ClassVar[frozenset[AnyAnalysis]] = TaintDetector.requires | {SSAAnalysis, ModuleStringsAnalysis}
+
+    def judge(self, ctx: PluginContext, function: nodes.Function, flow: TaintFlow, verdict: Verdict) -> Verdict:
+        ssa = ctx.get(SSAAnalysis, function)
+        defs = {i.result: i for block in ssa.blocks for i in block.instructions if i.result is not None}
+        target = flow_url(flow, defs, ctx.get(ModuleStringsAnalysis))
+        if target.relative:
+            return Verdict(flow, Status.REFUTED, f"the browser stays on the site: the target starts with {target.text!r}")
+        if target.origin is not None and verdict.status is Status.VULNERABILITY:
+            return Verdict(
+                flow,
+                Status.HOTSPOT,
+                f"the browser goes to the host fixed by {target.origin!r}, but the input chooses the path, "
+                "where that host may redirect again",
+            )
+        return verdict

src/coretrace_python/bundled/security/open_redirect/plugin.toml

@@ -1,7 +1,7 @@
 name = "open-redirect"
 version = "1.0.0"
 plugin_api = ">=1,<2"
-requires = ["taint.flows", "findings.refutation"]
+requires = ["taint.flows", "findings.refutation", "ir.ssa", "abstract.module_strings"]
 provides = ["vulnerability.open-redirect"]
 
 [entrypoint]

src/coretrace_python/bundled/security/ssrf/ssrf.py

@@ -1,37 +1,26 @@
 """Server-side request forgery: attacker-controlled input reaching a SSRF sink.
 
-How the URL is built, with ``+`` and f-strings, can prove part of its destination. Constant
-text fixing ``scheme://host`` and ending the authority with ``/``, ``?`` or ``#`` before
-any input keeps the input from choosing the host; the input still chooses the path, which
-may reach a sensitive resource of that host, and a redirect may lead elsewhere, so the
-flow is a hotspot. With the path fixed too, the input reaching the query or fragment
-only, and redirects disabled in the call, the destination is fixed and the flow is
-refuted. Anything unproven, a base of unknown value or a URL built in a helper function,
-stays a vulnerability.
+The rule draws its verdict from what the URL's constant text proves (``taint.urls``).
+A fixed host keeps the input from choosing it, but the input still chooses the path,
+which may reach a sensitive resource of that host, and a redirect may lead elsewhere: a
+hotspot. With the path fixed too, the input reaching the query or fragment only, and
+redirects disabled in the call, the destination is fixed: refuted. Anything unproven,
+a base of unknown value or a URL built in a helper function, stays a vulnerability.
 """
 
 from __future__ import annotations
 
-import re
 from typing import ClassVar
 
-from coretrace_python.abstract.strings import ModuleStringsAnalysis, leading_text
+from coretrace_python.abstract.strings import ModuleStringsAnalysis
 from coretrace_python.analysis import AnyAnalysis
 from coretrace_python.findings import Severity
 from coretrace_python.findings.refutation import Status, Verdict
 from coretrace_python.hir import nodes
 from coretrace_python.ir.ssa import SSAAnalysis
 from coretrace_python.plugins import PluginContext, TaintDetector
 from coretrace_python.taint import TaintFlow, TaintKind
-
-# ``scheme://`` and an authority ended by ``/``, ``?`` or ``#``. A backslash, which some
-# URL parsers take for a slash, does not end it here.
-_FIXED_HOST = re.compile(r"[A-Za-z][A-Za-z0-9+.-]*://[^/?#\\]+[/?#]")
-# The same with the path fixed too: what follows is query or fragment.
-_FIXED_PATH = re.compile(r"[A-Za-z][A-Za-z0-9+.-]*://[^/?#\\]+(?:/[^?#]*)?[?#]")
-# A client honours the one of these keywords it knows and rejects the other with a
-# ``TypeError``: either way, the call follows no redirect.
-_NO_REDIRECTS = ("allow_redirects", "follow_redirects")
+from coretrace_python.taint.urls import flow_url, redirects_disabled
 
 
 class SsrfPlugin(TaintDetector):
@@ -43,22 +32,18 @@ class SsrfPlugin(TaintDetector):
     requires: ClassVar[frozenset[AnyAnalysis]] = TaintDetector.requires | {SSAAnalysis, ModuleStringsAnalysis}
 
     def judge(self, ctx: PluginContext, function: nodes.Function, flow: TaintFlow, verdict: Verdict) -> Verdict:
-        if flow.through is not None:
-            return verdict  # the URL is built in the callee
         ssa = ctx.get(SSAAnalysis, function)
         defs = {i.result: i for block in ssa.blocks for i in block.instructions if i.result is not None}
-        text, _ = leading_text(flow.argument, defs, ctx.get(ModuleStringsAnalysis))
-        host = _FIXED_HOST.match(text)
-        if host is None:
+        url = flow_url(flow, defs, ctx.get(ModuleStringsAnalysis))
+        if url.origin is None:
             return verdict
-        arguments = flow.sink_arguments
-        no_redirects = arguments is not None and any(arguments.given(name) == (True, "False") for name in _NO_REDIRECTS)
-        if _FIXED_PATH.match(text) is not None:
+        no_redirects = redirects_disabled(flow.sink_arguments)
+        if url.path_fixed:
             if no_redirects:
-                return Verdict(flow, Status.REFUTED, f"the destination is fixed by {text!r} and redirects are disabled")
+                return Verdict(flow, Status.REFUTED, f"the destination is fixed by {url.text!r} and redirects are disabled")
             left = "a redirect may lead elsewhere"
         else:
             left = "the input chooses the path" + ("" if no_redirects else ", and a redirect may lead elsewhere")
         if verdict.status is not Status.VULNERABILITY:
             return verdict
-        return Verdict(flow, Status.HOTSPOT, f"the host is fixed by {host.group(0)!r}, but {left}")
+        return Verdict(flow, Status.HOTSPOT, f"the host is fixed by {url.origin!r}, but {left}")

src/coretrace_python/taint/urls.py

@@ -0,0 +1,70 @@
+"""What the constant text a URL starts with proves about its destination, and whether a
+call follows redirects: one proof that the rules reading URLs share, each drawing its
+own verdict from it. The text is what ``+`` and f-strings build before any other value
+(``abstract.strings``); the proof says nothing about the rest of the URL."""
+
+from __future__ import annotations
+
+import re
+from collections.abc import Mapping
+from dataclasses import dataclass
+
+from coretrace_python.abstract.strings import leading_text
+from coretrace_python.interprocedural import Arguments
+from coretrace_python.ir.model import Instruction, Value
+from coretrace_python.taint.engine import TaintFlow
+
+# ``scheme://`` and an authority ended by ``/``, ``?`` or ``#``. A backslash, which some
+# URL parsers take for a slash, does not end it here.
+_ORIGIN = re.compile(r"[A-Za-z][A-Za-z0-9+.-]*://[^/?#\\]+[/?#]")
+# The same with the path fixed too: what follows is query or fragment.
+_PATH = re.compile(r"[A-Za-z][A-Za-z0-9+.-]*://[^/?#\\]+(?:/[^?#]*)?[?#]")
+# A reference a browser resolves on the current site: ``/`` then a character that is not
+# a slash, a backslash, a space or a control character (``//`` and ``/\`` name another
+# host, and a browser drops tabs and newlines), a query or a fragment, or a relative path
+# whose first segment ends in the text without a ``:`` that would make it a scheme.
+_RELATIVE = re.compile(r"/[^/\\\x00-\x20\x7f]|[?#]|[^/\\?#:\x00-\x20\x7f][^/\\?#:]*[/?#]")
+# A client honours the one of these keywords it knows and rejects the other with a
+# ``TypeError``: either way, the call follows no redirect.
+_NO_REDIRECTS = ("allow_redirects", "follow_redirects")
+
+
+@dataclass(frozen=True)
+class UrlProof:
+    """What the constant ``text`` a URL starts with proves: the ``origin`` it fixes
+    (``scheme://authority`` and the character ending it), whether it fixes the path
+    too (``path_fixed``), and whether it starts a reference a browser resolves on the
+    current site (``relative``)."""
+
+    text: str
+    origin: str | None = None
+    path_fixed: bool = False
+    relative: bool = False
+
+
+def url_proof(text: str) -> UrlProof:
+    origin = _ORIGIN.match(text)
+    return UrlProof(
+        text,
+        origin.group(0) if origin is not None else None,
+        _PATH.match(text) is not None,
+        _RELATIVE.match(text) is not None,
+    )
+
+
+def flow_url(flow: TaintFlow, defs: Mapping[Value, Instruction], strings: Mapping[str, str]) -> UrlProof:
+    """The proof for the value ``flow`` passes to its sink, ``defs`` defining the values
+    of the function making the call. A flow reaching its sink in a callee proves nothing:
+    the value is built there."""
+
+    if flow.through is not None:
+        return UrlProof("")
+    text, _ = leading_text(flow.argument, defs, strings)
+    return url_proof(text)
+
+
+def redirects_disabled(arguments: Arguments | None) -> bool:
+    """Whether the call disables redirects explicitly (``allow_redirects=False`` or
+    ``follow_redirects=False``); absent, a variable or unpacked options do not."""
+
+    return arguments is not None and any(arguments.given(name) == (True, "False") for name in _NO_REDIRECTS)

tests/test_open_redirect_destinations.py

@@ -0,0 +1,116 @@
+"""An open-redirect flow is judged on the destination a browser resolves from the target,
+as far as ``+`` and f-strings build it, with its own criteria rather than SSRF's.
+
+- A target whose constant text starts a reference a browser resolves on the current site
+  (``/profile/``, ``?page=``, ``profile/``) keeps the browser on the site, where any
+  further redirect is the project's own code, analysed on its own: refuted.
+- A target whose constant text fixes an absolute ``scheme://host`` sends the browser to
+  that host, but the input chooses the path, where that host may redirect again: a
+  hotspot.
+- Anything else stays a vulnerability: ``"/" + next`` (``//evil.com`` is another host),
+  a backslash or a control character after the slash, a host left open, an unknown base,
+  a target built in a helper function.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from coretrace_python import engine
+from coretrace_python.findings import Finding
+from coretrace_python.source import SourceManager
+
+PLUGINS = [engine.BUNDLED_PLUGINS]
+
+FLASK = (
+    "from flask import Flask, redirect, request\n\n"
+    "app = Flask(__name__)\n"
+    "HOME = '/account/'\n\n"
+    "@app.route('/go/<uid>')\n"
+    "def go(uid):\n"
+    "    return {call}\n"
+)
+
+
+def check(text: str) -> tuple[Finding, ...]:
+    return engine.check(SourceManager().add_source("app.py", text), PLUGINS)
+
+
+def redirects(text: str) -> list[tuple[int, str]]:
+    return [(f.span.start_line, f.metadata["verdict"]) for f in check(text) if f.rule_id == "open-redirect"]
+
+
+@pytest.mark.parametrize(
+    "call",
+    [
+        "redirect('/profile/' + uid)",
+        "redirect(f'/users/{uid}/edit')",
+        "redirect('?page=' + uid)",
+        "redirect('#' + uid)",
+        "redirect('profile/' + uid)",
+        "redirect(HOME + uid)",
+    ],
+)
+def test_a_target_on_the_current_site_is_no_open_redirect(call: str) -> None:
+    assert redirects(FLASK.format(call=call)) == []
+
+
+@pytest.mark.parametrize(
+    "call",
+    [
+        "redirect('https://example.com/' + uid)",
+        "redirect(f'https://example.com/login?next={uid}')",
+    ],
+)
+def test_a_target_on_a_fixed_host_is_a_hotspot(call: str) -> None:
+    assert redirects(FLASK.format(call=call)) == [(8, "hotspot")]
+
+
+@pytest.mark.parametrize(
+    "call",
+    [
+        # ``next='/evil.com'`` makes ``//evil.com``, another host.
+        "redirect('/' + request.args['next'])",
+        "redirect('/\\\\' + uid)",
+        "redirect('/\\t' + uid)",
+        "redirect('https://example.com' + uid)",
+        "redirect('a:b/' + uid)",
+        "redirect('java' + uid)",
+        "redirect(request.args['next'])",
+    ],
+)
+def test_a_target_the_input_may_send_elsewhere_stays_a_vulnerability(call: str) -> None:
+    assert redirects(FLASK.format(call=call)) == [(8, "vulnerability")]
+
+
+def test_an_unknown_base_stays_a_vulnerability() -> None:
+    text = FLASK.format(call="redirect(HOME + uid)").replace("HOME = '/account/'\n", "from settings import HOME\n")
+
+    assert redirects(text) == [(8, "vulnerability")]
+
+
+def test_a_target_built_in_a_helper_function_stays_a_vulnerability() -> None:
+    text = FLASK.format(call="profile(uid)") + "\ndef profile(name):\n    return redirect('/profile/' + name)\n"
+
+    assert redirects(text) == [(8, "vulnerability")]
+
+
+def test_the_hotspot_evidence_names_the_fixed_host_and_the_further_redirect() -> None:
+    (finding,) = [f for f in check(FLASK.format(call="redirect('https://example.com/' + uid)")) if f.rule_id == "open-redirect"]
+
+    assert "'https://example.com/'" in finding.metadata["evidence"] and "redirect" in finding.metadata["evidence"]
+
+
+@pytest.mark.parametrize(
+    "target, verdicts",
+    [("'/items/' + pk", []), ("request.GET['next']", [(5, "vulnerability")])],
+)
+def test_django_redirects_follow_the_same_criteria(target: str, verdicts: list[tuple[int, str]]) -> None:
+    text = (
+        "from django.shortcuts import redirect\nfrom django.urls import path\n\n"
+        "def go(request, pk):\n"
+        f"    return redirect({target})\n\n"
+        "urlpatterns = [path('go/<pk>/', go)]\n"
+    )
+
+    assert redirects(text) == verdicts