Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
# 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).

### Advisories

- An advisory entry point may declare a `host` condition: the attacker must choose the host of the URL passed as the named argument. It is decided from the URL's constant text, the proof `ssrf` and `open-redirect` read. A fixed host with redirects disabled in the call leaves the call reachable, not exploitable. A fixed host with redirects followed keeps it exploitable, because a redirect may lead to a host the attacker controls, and the condition pending review says so. Input in the query does not choose the host. Anything else stays pending review, and so do the environment conditions. A `host` condition must name its argument and carry nothing else (#170).

## 0.14.0 (2026-09-26)

### Precision
Expand Down
4 changes: 3 additions & 1 deletion docs/plugins.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
19 changes: 18 additions & 1 deletion docs/usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |

Expand Down Expand Up @@ -386,6 +386,23 @@ a call passing a variable, or unpacking `*args` or `**kwargs`, leaves the condit
pending review. A `semantic` condition — the document's tags, the platform, the
template's origin — cannot be decided and is always pending review, never dropped.

A `host` condition says the attacker must choose the host of the URL passed as
`argument` (or at `position`), as credentials leaking to the host of a crafted URL
need: `{"kind": "host", "text": "…", "argument": "url", "position": 0}`. It is
decided from the URL's constant text, as `ssrf` and `open-redirect` read it, when the
attacker's input is passed in that argument. The outcome has three cases:

- Constant text fixes the host (`"https://api.example.com/" + path`) and the call
disables redirects (`allow_redirects=False`, `follow_redirects=False`): the call is
reachable, not exploitable.
- Constant text fixes the host but redirects may be followed: the call stays
exploitable. The fixed initial host does not clear the chain, since a redirect may
lead to a host the attacker controls, and the condition pending review says so.
Input in the query does not choose the host.
- Otherwise the condition stays pending review.

The other conditions, the environment included, stay pending as before.

Every dependency finding records the highest level of evidence established in its
`level` metadata: `declared` (the requirement allows a vulnerable version), `imported`
(a module of the package is imported somewhere), `reachable` (an entry point or affected
Expand Down
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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]
Expand Down
43 changes: 14 additions & 29 deletions src/coretrace_python/bundled/security/ssrf/ssrf.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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}")
7 changes: 6 additions & 1 deletion src/coretrace_python/dependency/advisories.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,11 +251,16 @@ def _condition(entry: Mapping[str, Any]) -> Condition:
default = entry.get("default", False)
if not isinstance(default, bool):
raise TypeError(f"condition default must be true or false, got {default!r}")
return Condition(
condition = Condition(
str(entry["kind"]),
str(entry["text"]),
None if entry.get("argument") is None else str(entry["argument"]),
tuple(str(v) for v in entry.get("values") or []),
position,
default,
)
if condition.kind == "host" and (
(condition.argument is None and condition.position is None) or condition.values or condition.default
):
raise ValueError("a host condition names the URL argument, by keyword or position, and nothing else")
return condition
41 changes: 36 additions & 5 deletions src/coretrace_python/dependency/correlation.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from __future__ import annotations

from collections.abc import Iterable, Mapping
from dataclasses import dataclass
from dataclasses import dataclass, replace

from coretrace_python.dependency.graph import (
DIRECT,
Expand All @@ -24,6 +24,7 @@
from coretrace_python.interprocedural import Arguments, CallSite, ExternalSymbol
from coretrace_python.semantic.symbols import SymbolId
from coretrace_python.taint import Sink, TaintFlow, TaintKind
from coretrace_python.taint.urls import UrlProof, redirects_disabled

Affected = Mapping[SymbolId, tuple[Advisory, ...]]

Expand Down Expand Up @@ -61,14 +62,33 @@ class ConditionCheck:
passed: str | None = None


def check_conditions(entry: AdvisoryEntryPoint | None, arguments: Arguments | None) -> ConditionCheck:
"""Decide ``entry``'s conditions against what the call's ``arguments`` denote."""
def check_conditions(
entry: AdvisoryEntryPoint | None,
arguments: Arguments | None,
url: UrlProof | None = None,
passed_as: frozenset[tuple[int | None, str | None]] = frozenset(),
) -> ConditionCheck:
"""Decide ``entry``'s conditions against what the call's ``arguments`` denote and, for
a ``host`` condition, what ``url`` proves of the value the attacker's input is passed
in (``passed_as``). A host the URL's constant text fixes rules the call out when the
call disables redirects; otherwise a redirect may still lead to a host the attacker
controls, and the condition stays pending with that uncertainty."""

if entry is None:
return ConditionCheck()
met: list[Condition] = []
pending: list[Condition] = []
for condition in entry.conditions:
if condition.kind == "host":
origin = url.origin if url is not None and _passed_in(condition, passed_as) else None
if origin is None:
pending.append(condition)
elif redirects_disabled(arguments):
return ConditionCheck(tuple(met), tuple(pending), condition, repr(origin))
else:
text = f"the host is fixed by {origin!r}, but a redirect may lead to a host the attacker controls"
pending.append(replace(condition, text=text))
continue
given = arguments.given(condition.argument, condition.position) if condition.checkable and arguments is not None else None
if given is None:
pending.append(condition)
Expand All @@ -84,6 +104,16 @@ def check_conditions(entry: AdvisoryEntryPoint | None, arguments: Arguments | No
return ConditionCheck(tuple(met), tuple(pending))


def _passed_in(condition: Condition, passed_as: frozenset[tuple[int | None, str | None]]) -> bool:
"""Whether a value passed as ``passed_as`` is the argument ``condition`` names."""

return any(
(keyword is not None and keyword == condition.argument)
or (keyword is None and position is not None and position == condition.position)
for position, keyword in passed_as
)


def ruled_out(module: str, site: CallSite, check: ConditionCheck) -> str:
"""Which call a contradicted condition rules out, and why: ``app:12
python.yaml.load(Loader=python.yaml.SafeLoader)``."""
Expand Down Expand Up @@ -120,9 +150,10 @@ def correlate(
flows: Iterable[TaintFlow],
verdicts: Verdicts | None,
affected: Affected,
urls: Mapping[TaintFlow, UrlProof] | None = None,
) -> tuple[Finding, ...]:
"""Exploitable-vulnerability findings for the non-refuted ADVISORY flows of a function,
one per advisory the sink is affected by."""
one per advisory the sink is affected by; ``urls`` holds what each flow's URL proves."""

findings: list[Finding] = []
for flow in flows:
Expand All @@ -136,7 +167,7 @@ def correlate(
entry = advisory.entry_point(flow.sink.symbol)
if entry is not None and not any(entry.exploitable_through(p, k) for p, k in flow.passed_as):
continue
check = check_conditions(entry, flow.sink_arguments)
check = check_conditions(entry, flow.sink_arguments, (urls or {}).get(flow), flow.passed_as)
if check.contradicted is None:
findings.append(_exploitable(function, flow, advisory, verdict, hotspot, check))
return tuple(findings)
Expand Down
6 changes: 4 additions & 2 deletions src/coretrace_python/dependency/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,8 +151,10 @@ class Condition:
``argument`` condition names the argument by keyword, and by ``position`` when it may
be passed positionally, and lists the ``values`` that satisfy it — symbols
(``python.yaml.FullLoader``) or constants as Python writes them (``True``);
``default`` says an absent argument means a vulnerable value. A ``semantic``
condition cannot be checked and is reported as pending review."""
``default`` says an absent argument means a vulnerable value. A ``host`` condition
says the attacker must choose the host of the URL passed as ``argument`` (or at
``position``); the engine decides it from what the URL's constant text proves. A
``semantic`` condition cannot be checked and is reported as pending review."""

kind: str
text: str
Expand Down
Loading
Loading