Skip to content

Commit bdeb00d

Browse files
authored
Merge pull request #147 from CoreTrace/feat/advisory-attacker-arguments
feat: let advisory entry points name the arguments the attacker must control
2 parents a2ea1a0 + 3f3aa13 commit bdeb00d

8 files changed

Lines changed: 315 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Precision
6+
7+
- An advisory entry point may name the `attacker_arguments` through which attacker input exploits it: by keyword, and by position when the argument may be passed positionally, a variadic one taking every later position. Attacker input in another argument leaves the call reachable instead of exploitable, so an advisory for `send_from_directory` that names its `path` no longer counts `download_name=`, and one for `requests.get` that names its `url` no longer counts the body. An argument unpacked with `*` or `**` may fill any of them: like an argument condition it leaves undecided, it does not rule the call out, so a wrapper forwarding `*args, **kwargs` still reaches the entry point. An entry point naming no argument counts every argument, as before. Each taint flow now records how its value is passed to the sink (#145).
8+
39
## 0.8.0 (2026-09-25)
410

511
### Precision

‎docs/usage.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,19 @@ it is reachable, called or not — `request.form['name']`, `for key in request.f
336336
`request.form.get('name')` all read it. A function reading it several times is reported
337337
once, where it first reads it.
338338

339+
Most vulnerabilities need attacker input in one argument: the path `send_from_directory`
340+
serves, not its `download_name`; the URL `requests.get` fetches, not its body. An entry
341+
point may name them in `attacker_arguments`, by keyword `argument` and, when it may be
342+
passed positionally, by `position`, counted as for argument conditions; `"variadic": true`
343+
extends a position to every later one, as `safe_join(directory, *pathnames)` takes
344+
(`{"position": 1, "variadic": true}`). A call is then exploitable only when attacker
345+
input reaches one of them, `"attacker_arguments": [{"argument": "url", "position": 0}]`
346+
for `requests.get`; attacker input in another argument leaves it reachable. An argument
347+
unpacked with `*args` or `**kwargs` may fill any of them: like an argument condition it
348+
leaves undecided, it does not rule the call out, so it counts, and a wrapper forwarding
349+
`*args, **kwargs` to the entry point still reaches it. An entry point without
350+
`attacker_arguments` is exploitable through any argument.
351+
339352
An `argument` condition is decided at each call. It names the argument by keyword and,
340353
when it may be passed positionally, by `position` (counted from 0, the receiver of a
341354
method excluded); `values` are what makes the call affected, written as the analyzer

‎src/coretrace_python/dependency/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
DEPENDENCY_FILES,
1313
Advisory,
1414
AdvisoryEntryPoint,
15+
AttackerArgument,
1516
Condition,
1617
DependencyAnalysis,
1718
DependencyGraph,
@@ -31,6 +32,7 @@
3132
"Advisory",
3233
"AdvisoryEntryPoint",
3334
"AdvisoryFileError",
35+
"AttackerArgument",
3436
"Condition",
3537
"DependencyAnalysis",
3638
"DependencyGraph",

‎src/coretrace_python/dependency/advisories.py‎

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,13 @@
1717
from pathlib import Path
1818
from typing import Any
1919

20-
from coretrace_python.dependency.graph import Advisory, AdvisoryEntryPoint, Condition, normalize
20+
from coretrace_python.dependency.graph import (
21+
Advisory,
22+
AdvisoryEntryPoint,
23+
AttackerArgument,
24+
Condition,
25+
normalize,
26+
)
2127
from coretrace_python.findings import Severity
2228
from coretrace_python.semantic.symbols import SymbolId
2329

@@ -155,6 +161,19 @@ def _entry_point_entry(entry_point: AdvisoryEntryPoint) -> dict[str, Any]:
155161
}
156162
if entry_point.read:
157163
entry["read"] = True
164+
if entry_point.attacker_arguments:
165+
entry["attacker_arguments"] = [_attacker_argument_entry(a) for a in entry_point.attacker_arguments]
166+
return entry
167+
168+
169+
def _attacker_argument_entry(argument: AttackerArgument) -> dict[str, Any]:
170+
entry: dict[str, Any] = {}
171+
if argument.argument is not None:
172+
entry["argument"] = argument.argument
173+
if argument.position is not None:
174+
entry["position"] = argument.position
175+
if argument.variadic:
176+
entry["variadic"] = True
158177
return entry
159178

160179

@@ -206,9 +225,25 @@ def _entry_point(entry: Mapping[str, Any]) -> AdvisoryEntryPoint:
206225
str(entry["justification"]),
207226
tuple(_condition(c) for c in entry.get("conditions") or []),
208227
read,
228+
tuple(_attacker_argument(a) for a in entry.get("attacker_arguments") or []),
209229
)
210230

211231

232+
def _attacker_argument(entry: Mapping[str, Any]) -> AttackerArgument:
233+
argument, position, variadic = entry.get("argument"), entry.get("position"), entry.get("variadic", False)
234+
if argument is not None and not isinstance(argument, str):
235+
raise TypeError(f"attacker argument name must be a string, got {argument!r}")
236+
if position is not None and (not isinstance(position, int) or isinstance(position, bool) or position < 0):
237+
raise TypeError(f"attacker argument position must be a non-negative integer, got {position!r}")
238+
if not isinstance(variadic, bool):
239+
raise TypeError(f"attacker argument variadic must be true or false, got {variadic!r}")
240+
if argument is None and position is None:
241+
raise ValueError("an attacker argument needs a name, a position or both")
242+
if variadic and position is None:
243+
raise ValueError("a variadic attacker argument needs the position it starts at")
244+
return AttackerArgument(argument, position, variadic)
245+
246+
212247
def _condition(entry: Mapping[str, Any]) -> Condition:
213248
position = entry.get("position")
214249
if position is not None and (not isinstance(position, int) or isinstance(position, bool) or position < 0):

‎src/coretrace_python/dependency/correlation.py‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,10 @@ def correlate(
133133
continue
134134
hotspot = verdict is not None and verdict.status is Status.HOTSPOT
135135
for advisory in affected.get(flow.sink.symbol, ()):
136-
check = check_conditions(advisory.entry_point(flow.sink.symbol), flow.sink_arguments)
136+
entry = advisory.entry_point(flow.sink.symbol)
137+
if entry is not None and not any(entry.exploitable_through(p, k) for p, k in flow.passed_as):
138+
continue
139+
check = check_conditions(entry, flow.sink_arguments)
137140
if check.contradicted is None:
138141
findings.append(_exploitable(function, flow, advisory, verdict, hotspot, check))
139142
return tuple(findings)

‎src/coretrace_python/dependency/graph.py‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,19 +166,52 @@ def checkable(self) -> bool:
166166
return self.kind == "argument"
167167

168168

169+
@dataclass(frozen=True)
170+
class AttackerArgument:
171+
"""An argument of an entry point that must carry the attacker's input: passed by
172+
keyword ``argument``, or at ``position`` (from 0, a method's receiver excluded) when it
173+
may be passed positionally; a ``variadic`` one, such as ``*pathnames``, also takes every
174+
later position."""
175+
176+
argument: str | None = None
177+
position: int | None = None
178+
variadic: bool = False
179+
180+
def receives(self, position: int | None, keyword: str | None) -> bool:
181+
"""Whether the argument passed at ``position`` or as ``keyword`` is this one. An
182+
argument unpacked with ``*`` or ``**`` has neither and may fill this one: like an
183+
argument condition it leaves undecided, it does not rule the call out, so it is."""
184+
185+
if keyword is not None:
186+
return keyword == self.argument
187+
if position is None:
188+
return True
189+
if self.position is None:
190+
return False
191+
return position == self.position or (self.variadic and position > self.position)
192+
193+
169194
@dataclass(frozen=True)
170195
class AdvisoryEntryPoint:
171196
"""A public API through which a project reaches an affected symbol, justified by the
172197
fixing commit or by a call path, with the conditions under which it is affected.
173198
174199
An entry point is reached by a call, or — when ``read`` is set, for an attribute whose
175200
getter runs the affected code, such as a lazily parsed request body — by any read of
176-
it or of an attribute of it."""
201+
it or of an attribute of it. When it names ``attacker_arguments``, attacker input
202+
exploits it only through one of them; otherwise through any argument."""
177203

178204
symbol: SymbolId
179205
justification: str
180206
conditions: tuple[Condition, ...] = ()
181207
read: bool = False
208+
attacker_arguments: tuple[AttackerArgument, ...] = ()
209+
210+
def exploitable_through(self, position: int | None, keyword: str | None) -> bool:
211+
"""Whether attacker input in the argument at ``position`` or ``keyword`` can
212+
exploit the vulnerability."""
213+
214+
return not self.attacker_arguments or any(a.receives(position, keyword) for a in self.attacker_arguments)
182215

183216

184217
DIRECT = "affected symbol, changed by the fix"

‎src/coretrace_python/taint/engine.py‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99

1010
from __future__ import annotations
1111

12-
from collections.abc import Callable, Mapping
12+
from collections.abc import Callable, Iterable, Mapping
1313
from dataclasses import dataclass, replace
1414
from types import MappingProxyType
1515
from typing import ClassVar
@@ -119,6 +119,9 @@ class TaintFlow:
119119
sink_location: SourceSpan | None = None
120120
# What the arguments of the sink call denote, to decide the conditions on them.
121121
sink_arguments: Arguments | None = None
122+
# How the tainted value is passed to the sink call: at a position, by a keyword, or
123+
# (None, None) unpacked with ``*`` or ``**``; ``f(x, key=x)`` passes it twice.
124+
passed_as: frozenset[tuple[int | None, str | None]] = frozenset()
122125

123126

124127
Key = Value | HeapLocation
@@ -559,6 +562,7 @@ def report(
559562
through,
560563
call.location if sink_location is None else sink_location,
561564
arguments,
565+
frozenset({(position, keyword)}),
562566
)
563567
)
564568

@@ -819,7 +823,18 @@ def propagate_taint(
819823
state, found = problem.evaluate(block, solution.incoming(block.id))
820824
taints.update(state)
821825
flows.extend(found)
822-
return TaintFacts(taints, tuple(dict.fromkeys(flows)))
826+
return TaintFacts(taints, _merged(flows))
827+
828+
829+
def _merged(flows: Iterable[TaintFlow]) -> tuple[TaintFlow, ...]:
830+
"""One flow per source, sink, kinds and tainted value, however many arguments of the
831+
sink call it is passed as."""
832+
833+
passed: dict[TaintFlow, frozenset[tuple[int | None, str | None]]] = {}
834+
for flow in flows:
835+
key = replace(flow, passed_as=frozenset())
836+
passed[key] = passed.get(key, frozenset()) | flow.passed_as
837+
return tuple(replace(flow, passed_as=ways) for flow, ways in passed.items())
823838

824839

825840
class TaintAnalysis(FunctionAnalysis[TaintFacts]):

0 commit comments

Comments
 (0)