mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-18 20:49:28 +00:00
feat(model): a region that spans the flow, not just a function
contributor-scan 2.4, closing the no-go task 2.2 recorded. VAmPI's SQL injection is a request parameter reaching `get_by_username` in api_views/users.py, passed to `User.get_user` in models/user_model.py, interpolated into a query string and executed there. Source and sink are in different functions and different modules. Regions were per-function, so neither region saw both ends and the engine was silent on the most ordinary shape a real injection bug has. `reachableByFlows` was never the limitation -- it follows calls already. The limitation was the QUESTION. Sinks were filtered to the labeled function's own body, so a crossing flow had no sink to end at. Three changes, each with a reason it is sound rather than convenient: **An entry point is the trust boundary.** parameter_sources was off for repository scans because a repository's parameters are not all attacker input. An entry point's are, by definition -- that is what makes it an entry point. The driver turns it on only for regions the mapper found and named, and leaves it off for walker fallbacks and for named functions that are not entry points. **Sinks may sit below the handler.** taint.sc admits sinks in methods reachable from the labeled method through the call graph, bounded at ENTRY_POINT_CALL_DEPTH = 3: handler -> service -> data access, which is where these bugs live. Unbounded would make the question about most of the repository rather than about this entry point, and make it unaffordable. **The finding names the sink, not the asker.** A crossing flow ends in another file and another function, so Verdict carries the evidence's own `path:line:function` and the site uses it. Reporting `models/user_model.py:73:get_by_username` would name a place that does not exist: line 73 is inside get_user. Also: labeled methods are now resolved by name AND file. `get_user` exists in more than one module of a real repository, which a corpus of one-file excerpts could never surface. Measured on the same checkout and budget as 2.2: VAmPI-SQLI missed -> model_entailed at models/user_model.py:73:get_user known found 0 of 2 before 2.2's fixes, 1 of 2 in 2.2, 2 of 2 now entailed 7 -> 8 query time 40.9s -> 37.4s peak RSS 869MB -> 827MB cost $0.0067, unchanged Asking the bigger question cost nothing measurable here: file scoping and call-graph following pull in opposite directions and roughly cancel at this size. That is one 520-line repository, and VAmPI is now the repository this change was developed against -- the exact overfitting shape Req 8 exists to catch. The depth bound and the entry-point rule mean nothing general until a checkout nobody tuned against says so. Gates byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
This commit is contained in:
co-authored by
Claude Opus 5
parent
177c7f3719
commit
ca7b5ba370
@@ -19,7 +19,9 @@ from .ladder import Rung, Verdict
|
||||
from .specs import TaintSpec
|
||||
|
||||
|
||||
def request_params(spec: TaintSpec, *, function: str = "", file: str = "", parameter_sources: bool = False) -> dict[str, object]:
|
||||
def request_params(
|
||||
spec: TaintSpec, *, function: str = "", file: str = "", parameter_sources: bool = False, call_depth: int = 0
|
||||
) -> dict[str, object]:
|
||||
"""The query parameters this arbiter sends. Shared with the batcher so there is one definition, not two.
|
||||
|
||||
Two copies of a parameter dict is precisely the duplication that produced five bugs of one shape in the
|
||||
@@ -37,17 +39,29 @@ def request_params(spec: TaintSpec, *, function: str = "", file: str = "", param
|
||||
"function": function,
|
||||
"file": file,
|
||||
"parameterSources": "true" if parameter_sources else "false",
|
||||
"callDepth": str(call_depth),
|
||||
}
|
||||
|
||||
|
||||
def verdict(cpg: CpgResult, spec: TaintSpec, *, function: str = "", file: str = "", parameter_sources: bool = False) -> Verdict | None:
|
||||
def verdict(
|
||||
cpg: CpgResult, spec: TaintSpec, *, function: str = "", file: str = "", parameter_sources: bool = False, call_depth: int = 0
|
||||
) -> Verdict | None:
|
||||
"""The strongest verdict the taint query supports for ``spec`` in ``function``, or ``None`` to stay at suspicion.
|
||||
|
||||
``parameter_sources`` additionally treats the labeled function's own parameters as untrusted. That is right
|
||||
for a function-level pair, where the function boundary *is* the trust boundary, and wrong for a whole
|
||||
repository, where most parameters carry internal values — so it is off by default and the caller opts in.
|
||||
An ENTRY POINT is the case where both are true at once: its parameters are attacker input by definition,
|
||||
which is what the driver uses to decide.
|
||||
|
||||
``call_depth`` admits sinks that many call-graph levels below the labeled method. Zero keeps the old
|
||||
function-local question; above zero is what lets a handler's parameter reach a sink in another module,
|
||||
which is the shape VAmPI's SQL injection has and the shape a function-scoped question cannot see.
|
||||
"""
|
||||
rows = cpg.run("taint", request_params(spec, function=function, file=file, parameter_sources=parameter_sources))
|
||||
rows = cpg.run(
|
||||
"taint",
|
||||
request_params(spec, function=function, file=file, parameter_sources=parameter_sources, call_depth=call_depth),
|
||||
)
|
||||
flows = _flows(rows, function=function)
|
||||
# A sink whose *shape* is safe is the fix, not the bug. `execute(sql, params)` binds rather than
|
||||
# interpolates and `printf("literal", x)` has a constant format string, so the flow that reaches them is
|
||||
@@ -66,7 +80,7 @@ def verdict(cpg: CpgResult, spec: TaintSpec, *, function: str = "", file: str =
|
||||
# Deterministic by construction: a total order, then take the first. Nothing depends on engine ordering.
|
||||
chosen = min(unsanitized or considered, key=lambda flow: _rank(flow, spec))
|
||||
rung = Rung.ENTAILED if not chosen["sanitized"] else Rung.CORROBORATED
|
||||
return Verdict(rung=rung, family=spec.family, witness=_witness(chosen))
|
||||
return Verdict(rung=rung, family=spec.family, witness=_witness(chosen), location=_location(chosen))
|
||||
|
||||
|
||||
def _flows(rows: object, *, function: str) -> list[dict[str, object]]:
|
||||
@@ -87,6 +101,7 @@ def _flows(rows: object, *, function: str) -> list[dict[str, object]]:
|
||||
"inLabeledScope": row.get("inLabeledScope"),
|
||||
"sinkLine": str(row.get("sinkLine", "")),
|
||||
"sinkMethod": str(row.get("sinkMethod", "")),
|
||||
"sinkFile": str(row.get("sinkFile", "")),
|
||||
"source": str(row.get("source", "")),
|
||||
"sanitized": bool(row.get("sanitized", False)),
|
||||
"length": _as_int(row.get("length")),
|
||||
@@ -155,7 +170,24 @@ def _rank(flow: Mapping[str, object], spec: TaintSpec) -> tuple[int, int, str, s
|
||||
|
||||
def _witness(flow: Mapping[str, object]) -> str:
|
||||
line = flow.get("sinkLine") or "?"
|
||||
return f"{flow['source']} -> {flow['sink']} (line {line}, {flow['length']} steps)"
|
||||
where = str(flow.get("sinkFile") or "")
|
||||
at = f" in {where}" if where else ""
|
||||
return f"{flow['source']} -> {flow['sink']}{at} (line {line}, {flow['length']} steps)"
|
||||
|
||||
|
||||
def _location(flow: Mapping[str, object]) -> str:
|
||||
"""``path:line:function`` of the SINK, when the engine reported a file. Empty leaves the region's own.
|
||||
|
||||
All three parts come from the sink, not from the region that asked. A flow that crosses modules ends in
|
||||
a different file AND a different function, and naming the entry point beside the sink's line describes a
|
||||
place that does not exist -- `models/user_model.py:73:get_by_username`, where line 73 is inside
|
||||
`get_user`.
|
||||
"""
|
||||
where = str(flow.get("sinkFile") or "")
|
||||
line = str(flow.get("sinkLine") or "")
|
||||
if not where or not line.lstrip("-").isdigit() or int(line) < 0:
|
||||
return ""
|
||||
return f"{where}:{line}:{flow.get('sinkMethod') or '?'}"
|
||||
|
||||
|
||||
__all__ = ["request_params", "verdict"]
|
||||
|
||||
Reference in New Issue
Block a user