From 14ce12984f850cd1b8143620c06474b53a901f24 Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 25 Sep 2026 01:36:46 +0200 Subject: [PATCH 1/2] test: specify that a sanitizer protects the flows through the function calling it --- tests/test_cache.py | 3 + tests/test_summary_sanitizers.py | 136 +++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 tests/test_summary_sanitizers.py diff --git a/tests/test_cache.py b/tests/test_cache.py index f73c9be..fd28cce 100644 --- a/tests/test_cache.py +++ b/tests/test_cache.py @@ -94,10 +94,13 @@ def test_findings_and_summaries_round_trip_through_json() -> None: span, None, arguments, + (((0, 4),), ()), + ((), ((1, 12),)), ), ), False, frozenset({SymbolId("python.builtins.input")}), + return_cleared=((0, 4),), ) functions = (ModuleFunction("f", span, "http"), ModuleFunction("g", span)) diff --git a/tests/test_summary_sanitizers.py b/tests/test_summary_sanitizers.py new file mode 100644 index 0000000..1ec9b98 --- /dev/null +++ b/tests/test_summary_sanitizers.py @@ -0,0 +1,136 @@ +"""A sanitizer called inside a project function protects the flows through it. + +A function summary says which parameters reach which external calls and the return value. +It also keeps, for each parameter, the taint kinds cleared on every path from it: +``escape`` clears ``HTML``, ``reverse`` clears ``REDIRECT``, and rendering a template the +project shows escaping clears ``HTML``. A caller's data then reaches the sink, or comes +back, without those kinds. A path that skips the sanitizer, or a kind the sanitizer does +not clear, still reaches it. Stores into objects and ``nonlocal`` writes clear nothing. +""" + +from __future__ import annotations + +from pathlib import Path + +from coretrace_python import engine +from coretrace_python.interprocedural.summaries import Dep +from coretrace_python.source import SourceManager + +DJANGO = ( + "import os\n" + "from django.http import HttpRequest, HttpResponse\n" + "from django.shortcuts import redirect\n" + "from django.urls import reverse\n" + "from django.utils.html import escape\n\n" +) + + +def rules(body: str) -> list[tuple[str, int]]: + findings = engine.check(SourceManager().add_source("views.py", DJANGO + body), [engine.BUNDLED_PLUGINS]) + return sorted((f.rule_id, f.span.start_line) for f in findings) + + +def test_two_paths_keep_only_what_both_cleared() -> None: + sanitized = Dep(frozenset({0, 1}), cleared=((0, 0b110), (1, 0b010))) + raw = Dep(frozenset({0}), cleared=((0, 0b011),)) + + assert (sanitized | raw).cleared == ((0, 0b010), (1, 0b010)) + assert Dep(frozenset({0, 2})).clearing(0b100).cleared == ((0, 0b100), (2, 0b100)) + assert (Dep(frozenset({0})) | Dep(frozenset({1}))).cleared == () + + +def test_a_sink_behind_a_sanitizer_in_a_helper_is_not_reached() -> None: + body = ( + "def page(value):\n" + " return HttpResponse(escape(value))\n\n" + "def view(request: HttpRequest):\n" + " return page(request.GET['name'])\n" + ) + + assert rules(body) == [] + + +def test_a_value_a_helper_returns_sanitized_stays_sanitized() -> None: + body = ( + "def clean(value):\n" + " return escape(value)\n\n" + "def view(request: HttpRequest):\n" + " return HttpResponse(clean(request.GET['name']))\n" + ) + + assert rules(body) == [] + + +def test_a_sanitizer_two_calls_deep_still_protects() -> None: + body = ( + "def clean(value):\n" + " return escape(value)\n\n" + "def page(value):\n" + " return HttpResponse(clean(value))\n\n" + "def view(request: HttpRequest):\n" + " return page(request.GET['name'])\n" + ) + + assert rules(body) == [] + + +def test_a_reversed_url_through_a_helper_is_no_open_redirect() -> None: + # healthchecks' ``_check_2fa``: ``next`` only ends up in the query of a local path. + body = ( + "def finish(request):\n" + " return redirect(reverse('home', query={'next': request.GET['next']}))\n\n" + "def login(request: HttpRequest):\n" + " return finish(request)\n" + ) + + assert rules(body) == [] + + +def test_a_path_around_the_sanitizer_still_reaches_the_sink() -> None: + body = ( + "def page(value, trusted):\n" + " shown = value if trusted else escape(value)\n" + " return HttpResponse(shown)\n\n" + "def view(request: HttpRequest):\n" + " return page(request.GET['name'], False)\n" + ) + + assert rules(body) == [("xss", 12)] + + +def test_a_sanitizer_clears_only_its_own_kinds() -> None: + body = ( + "def run(value):\n" + " os.system(escape(value))\n\n" + "def view(request: HttpRequest):\n" + " run(request.GET['cmd'])\n" + " return HttpResponse('ok')\n" + ) + + assert rules(body) == [("command-injection", 11)] + + +def test_a_template_rendered_in_a_helper_escapes_for_its_callers(tmp_path: Path) -> None: + files = { + "app/views.py": ( + "from django.http import HttpRequest, HttpResponse\n" + "from django.template.loader import render_to_string\n\n" + "def card(name):\n" + " return render_to_string('app/card.html', {'name': name})\n\n" + "def view(request: HttpRequest):\n" + " return HttpResponse(card(request.GET['name']))\n" + ), + "app/templates/app/card.html": "

{{ name }}

", + "app/templates/app/raw.html": "

{{ name|safe }}

", + } + for relative, text in files.items(): + path = tmp_path / relative + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + escaped = engine.analyze_project(tmp_path, [engine.BUNDLED_PLUGINS]).findings + (tmp_path / "app/views.py").write_text(files["app/views.py"].replace("card.html", "raw.html"), encoding="utf-8") + raw = engine.analyze_project(tmp_path, [engine.BUNDLED_PLUGINS]).findings + + assert [f.rule_id for f in escaped if f.rule_id == "xss"] == [] + assert [(f.rule_id, f.span.start_line) for f in raw if f.rule_id == "xss"] == [("xss", 8)] From c858a8085dc8c3849e97d5f130aaf19fc8535ed6 Mon Sep 17 00:00:00 2001 From: Hugo Date: Fri, 25 Sep 2026 01:36:47 +0200 Subject: [PATCH 2/2] fix: keep what sanitizers clear in function summaries --- CHANGELOG.md | 6 + docs/plugins.md | 2 +- docs/usage.md | 6 +- src/coretrace_python/cache.py | 17 +- src/coretrace_python/engine.py | 17 +- .../interprocedural/__init__.py | 8 + .../interprocedural/summaries.py | 159 +++++++++++++++--- src/coretrace_python/taint/engine.py | 44 ++--- src/coretrace_python/taint/models.py | 12 ++ tests/regression/expected/healthchecks.json | 56 ------ 10 files changed, 211 insertions(+), 116 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb31dc9..4d2864c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Precision + +- A sanitizer called inside a project function protects the flows through that function, for every caller. That covers a helper that escapes before answering, a view redirecting to `reverse(...)`, and a helper rendering a template the analyzer shows escaping. Function summaries keep, for each parameter, the taint kinds cleared on every path from it to an external call or to the return value. A path that skips the sanitizer, or a kind it does not clear, still reaches the sink. The cache format is bumped (#134). + ## 0.7.0 (2026-09-25) ### Precision diff --git a/docs/plugins.md b/docs/plugins.md index 064dba3..69a64fb 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -142,7 +142,7 @@ class BottleModels(ModelPlugin): |---|---| | `Source(symbol, label, kinds=ALL)` | A call or attribute whose value is attacker-controlled; `label` names it in messages (`http`, `stdin`, `argv`, `http-response`). `kinds` restricts what it can inject. | | `Sink(symbol, kinds, positions=(), keywords=())` | A call dangerous for `kinds`. `positions` limits a kind to the given argument indexes, and `keywords` to the given keyword names; a restricted kind reaches only the arguments listed there. SQL sinks read the statement only, and the SSRF sinks of HTTP clients read the URL, by position or as `url=`. A starred argument or `**kwargs` reaches no restricted kind. | -| `Sanitizer(symbol, kinds)` | A call whose result is safe for `kinds`. | +| `Sanitizer(symbol, kinds)` | A call whose result is safe for `kinds`, including when the call is inside a project function the data goes through. | | `EntryPoint(symbol, label, kinds=ALL)` | A decorator or a class base whose functions receive their parameters as `label` input, such as `flask.Flask.route`. | | `TypedParameter(symbol, label, kinds=ALL)` | A class whose annotated parameters carry `label` input, such as Django's `HttpRequest`. | | `NamedParameter(pattern, label, kinds)` | A regular expression on parameter names; matching parameters carry `kinds`, as `password` carries `CREDENTIAL`. | diff --git a/docs/usage.md b/docs/usage.md index 13f9d46..7c0a13f 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -101,7 +101,11 @@ confidence (`high`, `medium`, `low`). ### Taint flows Reported when attacker-controlled data reaches a sensitive call without passing through -a sanitizer for that kind of sink. +a sanitizer for that kind of sink. The sanitizer may also sit inside a project function +the data goes through, like a helper that escapes before it answers, or a view that +redirects to `reverse(...)`. It then protects every caller, on every path through the +function where it is called. What a function stores into an object or a `nonlocal` +variable is not treated that way yet. | Rule | Reached sink | |---|---| diff --git a/src/coretrace_python/cache.py b/src/coretrace_python/cache.py index c3d7313..996cc9f 100644 --- a/src/coretrace_python/cache.py +++ b/src/coretrace_python/cache.py @@ -25,6 +25,7 @@ from coretrace_python.interprocedural import ( Arguments, CallSite, + Cleared, ExternalCall, ExternalSymbol, FunctionSummary, @@ -41,7 +42,7 @@ from coretrace_python.semantic.symbols import SymbolId from coretrace_python.source import SourceId, SourceSpan -CACHE_FORMAT = 9 +CACHE_FORMAT = 10 @dataclass(frozen=True) @@ -237,6 +238,8 @@ def _encode_call(call: ExternalCall) -> dict[str, Any]: "location": _encode_span(call.location), "call_site": None if call.call_site is None else _encode_span(call.call_site), "given": _encode_arguments(call.arguments), + "argument_cleared": [_encode_cleared(c) for c in call.argument_cleared], + "keyword_cleared": [_encode_cleared(c) for c in call.keyword_cleared], } @@ -249,9 +252,19 @@ def _decode_call(data: Mapping[str, Any]) -> ExternalCall: _decode_span(data["location"]), None if site is None else _decode_span(site), _decode_arguments(data["given"]), + tuple(_decode_cleared(c) for c in data["argument_cleared"]), + tuple(_decode_cleared(c) for c in data["keyword_cleared"]), ) +def _encode_cleared(cleared: Cleared) -> list[list[int]]: + return [[parameter, bits] for parameter, bits in cleared] + + +def _decode_cleared(data: Any) -> Cleared: + return tuple((_integer(parameter), _integer(bits)) for parameter, bits in data) + + def _encode_summary(summary: FunctionSummary) -> dict[str, Any]: return { "name": summary.name, @@ -275,6 +288,7 @@ def _encode_summary(summary: FunctionSummary) -> dict[str, Any]: for w in summary.nonlocal_writes ], "static": summary.static, + "return_cleared": _encode_cleared(summary.return_cleared), } @@ -303,6 +317,7 @@ def _decode_summary(data: Mapping[str, Any]) -> FunctionSummary: for w in data["nonlocal_writes"] ), bool(data["static"]), + _decode_cleared(data["return_cleared"]), ) diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index d41776f..131f25f 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -70,6 +70,7 @@ CallGraph, CallGraphAnalysis, CallSite, + ClearingAnalysis, FunctionSummary, ModuleFunction, ModuleGraph, @@ -149,6 +150,7 @@ DependencyAnalysis, RegisteredRoutes, EscapedTemplates, + ClearingAnalysis, ) @@ -188,6 +190,7 @@ class ResultsEvicted(TransformationPass): DependencyAnalysis, RegisteredRoutes, EscapedTemplates, + ClearingAnalysis, } ) @@ -246,7 +249,9 @@ def build_manager( """A manager with every engine analysis registered and the engine inputs provided.""" manager = _register_all(module) - manager.provide(SecurityModelAnalysis, (models or SecurityModelRegistry()).freeze()) + table = (models or SecurityModelRegistry()).freeze() + manager.provide(SecurityModelAnalysis, table) + manager.provide(ClearingAnalysis, table.clearing()) manager.provide(ProjectSummaries, SummaryIndex()) manager.provide(RegisteredRoutes, _routes_of(manager)) return manager @@ -294,7 +299,9 @@ def analyze_file(source: SourceFile, plugin_roots: Sequence[Path]) -> FileAnalys manager = _register_all(build_hir(source)) registry = load_plugins(plugin_roots, manager) - manager.provide(SecurityModelAnalysis, plugin_models(loaded.plugin for loaded in registry)) + table = plugin_models(loaded.plugin for loaded in registry) + manager.provide(SecurityModelAnalysis, table) + manager.provide(ClearingAnalysis, table.clearing()) manager.provide(ProjectSummaries, SummaryIndex()) manager.provide(RegisteredRoutes, _routes_of(manager)) findings, supported = _check_module(manager, tuple(loaded.plugin for loaded in registry)) @@ -408,9 +415,11 @@ def analyze_project( for symbol, registered in _routes_of(analysable[name]).items(): routes.setdefault(symbol, registered) escaped = escaped_templates(root) + clearing = models.clearing(escaped) for manager in analysable.values(): manager.provide(RegisteredRoutes, routes) manager.provide(EscapedTemplates, escaped) + manager.provide(ClearingAnalysis, clearing) configuration = _configuration_key(registry, plugins, models, advisories, dependencies, routes, escaped) keys = module_keys( @@ -622,11 +631,13 @@ def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: affected = affected_symbols(dependencies, advisories) models = plugin_models(all_plugins).extended(*advisory_sinks(affected)) routes = _decode_routes(batch.routes) + escaped = frozenset(batch.escaped) for manager in managers.values(): manager.provide(SecurityModelAnalysis, models) manager.provide(DependencyAnalysis, dependencies) manager.provide(RegisteredRoutes, routes) - manager.provide(EscapedTemplates, frozenset(batch.escaped)) + manager.provide(EscapedTemplates, escaped) + manager.provide(ClearingAnalysis, models.clearing(escaped)) module_plugins = tuple(p for p in all_plugins if not isinstance(p, ProjectPlugin)) results = _analyse_managers(managers, decode_index(batch.seed), module_plugins, affected) return {name: encode(entry) for name, entry in results.items()} diff --git a/src/coretrace_python/interprocedural/__init__.py b/src/coretrace_python/interprocedural/__init__.py index d6bd03b..99642d2 100644 --- a/src/coretrace_python/interprocedural/__init__.py +++ b/src/coretrace_python/interprocedural/__init__.py @@ -20,6 +20,9 @@ project_symbol, ) from coretrace_python.interprocedural.summaries import ( + Cleared, + Clearing, + ClearingAnalysis, ExternalCall, FunctionSummary, Mutation, @@ -28,6 +31,7 @@ SummaryAnalysis, SummaryIndex, SummaryTable, + cleared_by, ) __all__ = [ @@ -35,6 +39,9 @@ "CallGraph", "CallGraphAnalysis", "CallSite", + "Cleared", + "Clearing", + "ClearingAnalysis", "ExternalCall", "ExternalSymbol", "FunctionSummary", @@ -51,6 +58,7 @@ "Target", "UnknownTarget", "build_module_graph", + "cleared_by", "discover_files", "discover_sources", "project_symbol", diff --git a/src/coretrace_python/interprocedural/summaries.py b/src/coretrace_python/interprocedural/summaries.py index cc480ff..33e8d75 100644 --- a/src/coretrace_python/interprocedural/summaries.py +++ b/src/coretrace_python/interprocedural/summaries.py @@ -4,16 +4,19 @@ symbols its parameters reach, directly or through calls to other known functions. It is computed as a dependence data-flow problem over the SSA form and iterated to a fixpoint over the call graph, so recursion converges to the least solution. Summaries carry no -security knowledge: the taint engine decides which external symbols matter. +security knowledge of their own: the taint engine decides which external symbols +matter. They only keep, per parameter, the bits a ``Clearing`` the engine provides says +the calls on the way cleared, on every path, so a caller's data reaches a sink or comes +back without the kinds a sanitizer inside the function removed. """ from __future__ import annotations import dataclasses -from collections.abc import Mapping +from collections.abc import Iterable, Mapping from dataclasses import dataclass from types import MappingProxyType -from typing import ClassVar +from typing import ClassVar, TypeVar from coretrace_python.abstract import ( ATTRIBUTES, @@ -69,22 +72,90 @@ Dependencies = frozenset[int] NONE: Dependencies = frozenset() +_T = TypeVar("_T") +# Per parameter, the taint kind bits cleared on every path from it, sorted, none empty. +Cleared = tuple[tuple[int, int], ...] @dataclass(frozen=True) class Dep: - """What a value depends on: parameter indices and results of external symbols.""" + """What a value depends on: parameter indices, results of external symbols, and for + each parameter the bits cleared on every path from it to the value.""" parameters: Dependencies = NONE externals: frozenset[SymbolId] = frozenset() + cleared: Cleared = () def __or__(self, other: Dep) -> Dep: - return Dep(self.parameters | other.parameters, self.externals | other.externals) + parameters = self.parameters | other.parameters + if not self.cleared and not other.cleared: + return Dep(parameters, self.externals | other.externals) + own, theirs = dict(self.cleared), dict(other.cleared) + # A parameter both sides depend on keeps what both paths cleared. + cleared = { + p: (own.get(p, 0) & theirs.get(p, 0)) + if p in self.parameters and p in other.parameters + else own.get(p, 0) | theirs.get(p, 0) + for p in parameters + } + return Dep(parameters, self.externals | other.externals, _cleared(cleared)) + + def clearing(self, bits: int) -> Dep: + """This dependence after a call clearing ``bits`` from what it returns.""" + + if not bits or not self.parameters: + return self + own = dict(self.cleared) + return Dep(self.parameters, self.externals, _cleared({p: own.get(p, 0) | bits for p in self.parameters})) + + +def _cleared(bits: Mapping[int, int]) -> Cleared: + return tuple(sorted((p, b) for p, b in bits.items() if b)) + + +def cleared_by(cleared: Cleared, parameter: int) -> int: + """The bits ``cleared`` records for ``parameter``, none when it has no entry.""" + + return next((bits for p, bits in cleared if p == parameter), 0) EMPTY = Dep() +@dataclass(frozen=True) +class Clearing: + """What an external call clears from the data it returns, as taint kind bits the + summaries carry without reading them. The engine derives it from the security models: + a sanitizer clears its kinds; a template render clears the bits of its entry when the + template it names is one the project shows escaping (``escaped`` holds the names the + way call sites record a constant).""" + + sanitizers: Mapping[SymbolId, int] = dataclasses.field(default_factory=dict) + renders: Mapping[SymbolId, tuple[int, str, int]] = dataclasses.field(default_factory=dict) + escaped: frozenset[str] = frozenset() + + def clears(self, symbol: SymbolId, arguments: Arguments) -> int: + bits = self.sanitizers.get(symbol, 0) + render = self.renders.get(symbol) + if render is not None: + position, keyword, rendered = render + named = arguments.given(keyword, position) + if named is not None and named[0] and named[1] in self.escaped: + bits |= rendered + return bits + + +class ClearingAnalysis(Analysis[Clearing]): + """What calls clear, provided by the engine from the security models; nothing on its + own, so a summary then keeps every kind.""" + + name: ClassVar[str] = "interprocedural.clearing" + + @classmethod + def compute(cls, ctx: AnalysisContext) -> Clearing: + return Clearing() + + @dataclass(frozen=True) class ExternalCall: """An external symbol reached from this function; dependencies are parameter indices. @@ -98,6 +169,19 @@ class ExternalCall: call_site: SourceSpan | None # What the arguments of the call at ``location`` denote, where it is made. arguments: Arguments = dataclasses.field(default_factory=Arguments) + # What was cleared on the way to each argument and keyword, in the same order. + argument_cleared: tuple[Cleared, ...] = () + keyword_cleared: tuple[Cleared, ...] = () + + def positional(self) -> Iterable[tuple[Dependencies, Cleared]]: + """Each positional argument's dependencies, with what was cleared on their way.""" + + return _aligned(self.argument_dependencies, self.argument_cleared) + + def named(self) -> Iterable[tuple[str | None, Dependencies, Cleared]]: + """Each keyword's name and dependencies, with what was cleared on their way.""" + + return ((name, deps, c) for (name, deps), c in _aligned(self.keyword_dependencies, self.keyword_cleared)) @dataclass(frozen=True) @@ -134,6 +218,8 @@ class FunctionSummary: nonlocal_writes: tuple[NonlocalWrite, ...] = () # A ``@staticmethod``: ``Class.method(...)`` passes no receiver. static: bool = False + # What was cleared on the way from each parameter to the return value. + return_cleared: Cleared = () class SummaryIndex: @@ -191,6 +277,7 @@ def __init__( table: Mapping[str, FunctionSummary], project: Mapping[SymbolId, FunctionSummary] | None = None, heap: HeapFacts | None = None, + clearing: Clearing | None = None, ) -> None: self.name = name self.function = function @@ -198,6 +285,7 @@ def __init__( self.table = table self.project = project or {} self.heap = heap or HeapFacts({}) + self.clearing = clearing or Clearing() self.blocks = {block.id: block for block in function.blocks} self.defs: dict[Value, Instruction] = { i.result: i for block in function.blocks for i in block.instructions if i.result @@ -338,15 +426,10 @@ def call(self, call: Call, state: dict[Key, Dep]) -> Dep: bound = () if project.static else self.receiver(call, project.name) arguments = (*(self.deep(r, state) for r in bound), *arguments) return self.known(project, arguments, keywords, everything, call, state, bound) - self.record( - target.symbol, - tuple(a.parameters for a in arguments), - tuple((name, deps.parameters) for name, deps in named), - call.location, - None, - self.graph.arguments_at(self.name, call.location), - ) - return everything | Dep(externals=frozenset({target.symbol})) + given = self.graph.arguments_at(self.name, call.location) + self.record(target.symbol, arguments, named, call.location, None, given) + cleared = everything.clearing(self.clearing.clears(target.symbol, given)) + return cleared | Dep(externals=frozenset({target.symbol})) if isinstance(target, KnownFunction): made = self.defs.get(call.callee) if isinstance(made, MakeFunction): @@ -414,17 +497,18 @@ def known( if callee.unsupported: return everything - def mapped(indices: Dependencies) -> Dep: + def mapped(indices: Dependencies, cleared: Cleared = ()) -> Dep: result = EMPTY for index in indices: - result |= arguments[index] if index < len(arguments) else keywords + part = arguments[index] if index < len(arguments) else keywords + result |= part.clearing(cleared_by(cleared, index)) return result for reached in callee.external_calls: self.record( reached.symbol, - tuple(mapped(d).parameters for d in reached.argument_dependencies), - tuple((name, mapped(d).parameters) for name, d in reached.keyword_dependencies), + tuple(mapped(d, c) for d, c in reached.positional()), + tuple((name, mapped(d, c)) for name, d, c in reached.named()), reached.location, call.location, reached.arguments, @@ -435,13 +519,13 @@ def mapped(indices: Dependencies) -> Dep: if mutation.parameter < len(values): deps = mapped(mutation.dependencies) | Dep(externals=mutation.externals) self.store(state, values[mutation.parameter], mutation.field, deps) - return mapped(callee.return_dependencies) | Dep(externals=callee.return_externals) + return mapped(callee.return_dependencies, callee.return_cleared) | Dep(externals=callee.return_externals) def record( self, symbol: SymbolId, - arguments: tuple[Dependencies, ...], - keywords: tuple[tuple[str | None, Dependencies], ...], + arguments: tuple[Dep, ...], + keywords: tuple[tuple[str | None, Dep], ...], location: SourceSpan, call_site: SourceSpan | None, given: Arguments, @@ -450,14 +534,30 @@ def record( previous = self.external.get(key) if previous is not None: arguments = tuple( - a | b for a, b in zip(previous.argument_dependencies, arguments, strict=False) + a | Dep(b, cleared=c) for a, (b, c) in zip(arguments, previous.positional(), strict=False) ) # The same call records the same keywords, in the same order. keywords = tuple( - (name, deps | old) - for (name, deps), (_, old) in zip(keywords, previous.keyword_dependencies, strict=False) + (name, deps | Dep(old, cleared=c)) + for (name, deps), (_, old, c) in zip(keywords, previous.named(), strict=False) ) - self.external[key] = ExternalCall(symbol, arguments, keywords, location, call_site, given) + self.external[key] = ExternalCall( + symbol, + tuple(a.parameters for a in arguments), + tuple((name, deps.parameters) for name, deps in keywords), + location, + call_site, + given, + tuple(a.cleared for a in arguments), + tuple(deps.cleared for _, deps in keywords), + ) + + +def _aligned(dependencies: tuple[_T, ...], cleared: tuple[Cleared, ...]) -> Iterable[tuple[_T, Cleared]]: + """Each dependency with what was cleared on its way; nothing when not recorded.""" + + missing = len(dependencies) - len(cleared) + return zip(dependencies, (*cleared, *(() for _ in range(missing))), strict=True) def summarize( @@ -469,8 +569,9 @@ def summarize( project: Mapping[SymbolId, FunctionSummary] | None = None, heap: HeapFacts | None = None, static: bool = False, + clearing: Clearing | None = None, ) -> FunctionSummary: - problem = _DependenceProblem(name, function, graph, table, project, heap) + problem = _DependenceProblem(name, function, graph, table, project, heap, clearing) solution = solve(problem, cfg) problem.external = {} problem.returns = EMPTY @@ -502,19 +603,21 @@ def summarize( if deps.parameters or deps.externals ), static=static, + return_cleared=problem.returns.cleared, ) class SummaryAnalysis(Analysis[SummaryTable]): name: ClassVar[str] = "interprocedural.summaries" requires: ClassVar[frozenset[AnyAnalysis]] = frozenset( - {CallGraphAnalysis, SSAAnalysis, CFGAnalysis, ProjectSummaries, HeapAnalysis} + {CallGraphAnalysis, SSAAnalysis, CFGAnalysis, ProjectSummaries, HeapAnalysis, ClearingAnalysis} ) @classmethod def compute(cls, ctx: AnalysisContext) -> SummaryTable: graph = ctx.get(CallGraphAnalysis) project = ctx.get(ProjectSummaries).summaries + clearing = ctx.get(ClearingAnalysis) table: dict[str, FunctionSummary] = {} supported: dict[str, tuple[FunctionIR, CFG, HeapFacts]] = {} for name, function in graph.definitions.items(): @@ -533,7 +636,7 @@ def compute(cls, ctx: AnalysisContext) -> SummaryTable: changed = False for name, (ssa, cfg, heap) in supported.items(): updated = summarize( - name, ssa, cfg, graph, table, project, heap, _is_staticmethod(graph.definitions[name]) + name, ssa, cfg, graph, table, project, heap, _is_staticmethod(graph.definitions[name]), clearing ) if updated != table[name]: table[name] = updated diff --git a/src/coretrace_python/taint/engine.py b/src/coretrace_python/taint/engine.py index 657116c..4e00f80 100644 --- a/src/coretrace_python/taint/engine.py +++ b/src/coretrace_python/taint/engine.py @@ -30,6 +30,8 @@ Arguments, CallGraph, CallGraphAnalysis, + Cleared, + Clearing, ExternalSymbol, FunctionSummary, KnownFunction, @@ -38,6 +40,7 @@ SummaryAnalysis, SummaryIndex, SummaryTable, + cleared_by, project_symbol, ) from coretrace_python.ir.lowering import analyzable_functions, is_body_function @@ -153,12 +156,11 @@ def __init__( heap: HeapFacts | None = None, seeds: Mapping[HeapLocation, Taint] | None = None, module: str = "", - escaped: frozenset[str] = frozenset(), + clearing: Clearing | None = None, ) -> None: self.name = name self.module = module - # As call sites record a constant argument: the way Python writes it. - self.escaped = frozenset(repr(name) for name in escaped) + self.clearing = clearing or Clearing() self.function = function self.models = models self.graph = graph @@ -411,11 +413,9 @@ def external( self.report( flows, sink, self.rendered(argument, state), argument, call, None, None, None, given, keyword=name ) - sanitizer = self.models.sanitizer(symbol) - if sanitizer is not None: - return everything.without(sanitizer.kinds) - if self.renders_escaped(symbol, given): - return everything.without(TaintKind.HTML) + cleared = self.clearing.clears(symbol, given) + if cleared: + return everything.without(TaintKind(cleared)) # A method on a tainted object returns tainted data (``request.args.get``). everything = everything.join(state.get(call.callee, Taint.none())) source = self.models.source_covering(symbol) @@ -423,14 +423,6 @@ def external( return everything.join(Taint(source.kinds, frozenset({source}))) return everything - def renders_escaped(self, symbol: SymbolId, given: Arguments) -> bool: - """Whether the call renders, by a name written in the call, a template the - project shows escaping everything it renders.""" - - render = self.models.template_render(symbol) - named = given.given(render.keyword, render.position) if render is not None else None - return named is not None and named[0] and named[1] in self.escaped - def guarded(self, sink: Sink | None, arguments: Arguments) -> Sink | None: """The sink as this call makes it: an argument declared safe (``yaml.load`` with ``Loader=SafeLoader``) takes off the kinds it covers, and no kind left, no sink.""" @@ -501,11 +493,11 @@ def known( spread = (*call.starred, *(value for _, value in call.keywords)) values = (*receiver, *call.arguments, *captured) - def mapped(deps: frozenset[int]) -> tuple[Taint, Value | None]: + def mapped(deps: frozenset[int], cleared: Cleared = ()) -> tuple[Taint, Value | None]: taint, witness = Taint.none(), None for index in sorted(deps): positional = index < len(arguments) - part = arguments[index] if positional else keywords + part = (arguments[index] if positional else keywords).without(TaintKind(cleared_by(cleared, index))) if part and witness is None and (positional or spread): witness = values[index] if positional else spread[0] taint = taint.join(part) @@ -515,12 +507,12 @@ def mapped(deps: frozenset[int]) -> tuple[Taint, Value | None]: sink = self.guarded(self.models.sink(reached.symbol), reached.arguments) if sink is None: continue - for position, deps in enumerate(reached.argument_dependencies): - taint, witness = mapped(deps) + for position, (deps, cleared) in enumerate(reached.positional()): + taint, witness = mapped(deps, cleared) if witness is not None: self.report(flows, sink, taint, witness, call, through, reached.location, position, reached.arguments) - for name, deps in reached.keyword_dependencies: - taint, witness = mapped(deps) + for name, deps, cleared in reached.named(): + taint, witness = mapped(deps, cleared) if witness is not None: self.report( flows, sink, taint, witness, call, through, reached.location, None, reached.arguments, keyword=name @@ -533,7 +525,7 @@ def mapped(deps: frozenset[int]) -> tuple[Taint, Value | None]: if stored_source is not None: stored = stored.join(Taint(stored_source.kinds, frozenset({stored_source}))) self.store(state, values[mutation.parameter], mutation.field, stored) - result = mapped(summary.return_dependencies)[0] + result = mapped(summary.return_dependencies, summary.return_cleared)[0] for symbol in sorted(summary.return_externals, key=str): returned_source = self.models.source(symbol) if returned_source is not None: @@ -816,9 +808,9 @@ def propagate_taint( heap: HeapFacts | None = None, seeds: Mapping[HeapLocation, Taint] | None = None, module: str = "", - escaped: frozenset[str] = frozenset(), + clearing: Clearing | None = None, ) -> TaintFacts: - problem = _TaintProblem(name, function, models, graph, summaries, parameters, project, heap, seeds, module, escaped) + problem = _TaintProblem(name, function, models, graph, summaries, parameters, project, heap, seeds, module, clearing) solution = solve(problem, cfg) taints: dict[Key, Taint] = {} flows: list[TaintFlow] = [] @@ -883,7 +875,7 @@ def sources_of(member: nodes.Function) -> Mapping[int, Source]: heap, seeds, ctx.module.name, - ctx.get(EscapedTemplates), + models.clearing(ctx.get(EscapedTemplates)), ) diff --git a/src/coretrace_python/taint/models.py b/src/coretrace_python/taint/models.py index 03de1fe..77c9a6a 100644 --- a/src/coretrace_python/taint/models.py +++ b/src/coretrace_python/taint/models.py @@ -16,6 +16,7 @@ from typing import ClassVar from coretrace_python.analysis import Analysis, AnalysisContext, MissingInputError +from coretrace_python.interprocedural import Clearing from coretrace_python.semantic.symbols import SymbolId @@ -334,6 +335,17 @@ def template_render(self, symbol: SymbolId) -> TemplateRender | None: found = self._by_symbol[TemplateRender].get(symbol) return found if isinstance(found, TemplateRender) else None + def clearing(self, escaped: frozenset[str] = frozenset()) -> Clearing: + """What calls clear from the data they return: every sanitizer its kinds, every + template render ``HTML`` when it names one of the ``escaped`` templates.""" + + return Clearing( + MappingProxyType({s.symbol: s.kinds.value for s in self.sanitizers}), + MappingProxyType({r.symbol: (r.position, r.keyword, TaintKind.HTML.value) for r in self.template_renders}), + # As call sites record a constant argument: the way Python writes it. + frozenset(repr(name) for name in escaped), + ) + class SecurityModelRegistry: """Mutable collection point for the models plugins register.""" diff --git a/tests/regression/expected/healthchecks.json b/tests/regression/expected/healthchecks.json index bcf1649..658f82a 100644 --- a/tests/regression/expected/healthchecks.json +++ b/tests/regression/expected/healthchecks.json @@ -74,47 +74,12 @@ "function": "_redirect_after_login", "verdict": "hotspot" }, - { - "file": "hc/accounts/views.py", - "line": 170, - "rule": "open-redirect", - "function": "login", - "verdict": "hotspot" - }, - { - "file": "hc/accounts/views.py", - "line": 281, - "rule": "open-redirect", - "function": "check_token", - "verdict": "hotspot" - }, - { - "file": "hc/accounts/views.py", - "line": 281, - "rule": "open-redirect", - "function": "check_token", - "verdict": "hotspot" - }, { "file": "hc/accounts/views.py", "line": 314, "rule": "hardcoded-credential", "function": "profile" }, - { - "file": "hc/accounts/views.py", - "line": 637, - "rule": "open-redirect", - "function": "change_email_verify", - "verdict": "vulnerability" - }, - { - "file": "hc/accounts/views.py", - "line": 637, - "rule": "open-redirect", - "function": "change_email_verify", - "verdict": "vulnerability" - }, { "file": "hc/api/models.py", "line": 434, @@ -170,20 +135,6 @@ "function": "ping_body", "verdict": "hotspot" }, - { - "file": "hc/api/views.py", - "line": 778, - "rule": "xss", - "function": "badge", - "verdict": "hotspot" - }, - { - "file": "hc/api/views.py", - "line": 811, - "rule": "xss", - "function": "check_badge", - "verdict": "vulnerability" - }, { "file": "hc/front/tests/test_webhook_validator.py", "line": 22, @@ -347,13 +298,6 @@ "rule": "hardcoded-credential", "function": "test_it_obeys_rate_limit" }, - { - "file": "hc/integrations/po/views.py", - "line": 59, - "rule": "open-redirect", - "function": "add", - "verdict": "hotspot" - }, { "file": "hc/integrations/pushbullet/tests/test_add_complete.py", "line": 22,