mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 19:59:26 +00:00
fix(model-grounded-detection): two modelling bugs the first live Joern run exposed
Both were found by running the real engine, and neither was visible offline.
1. A sink was its own sanitizer. The shipped facts mark some sanitizers with a
*shape* qualifier -- parameterized ("execute(sql, params) binds rather than
interpolates") and literal_format_arg ("printf with a constant format string
is safe"). These describe a safe FORM OF THE SINK CALL, not a call that
cleans a value on the path. Flattening them into the sanitizer list made
`execute` both the injection sink and its own sanitizer, so every SQL flow
would have reported as already-clean and nothing could ever be entailed.
Fixed as a class, not an instance: any fact carrying a shape qualifier is
excluded from `sanitizers` and kept in `safe_shape_sinks` for the shape test
task 4.2 will model properly. Fixing only the Python `parameterized` case
would have left C's entire printf family -- which the generalised test then
caught immediately.
2. labeled_family lost its deterministic derivation in group 1. The deleted
classifier resolved a pair's family by declared label, else obligation, else
mechanism, else CWE, and only asked an LLM when every link failed -- which
for a labeled corpus was never. Group 1 reduced this to the declared label
alone, which silently emptied every family whose pairs are labeled by CWE:
the injection slice went to zero pairs. The chain is restored, still with no
model call, and the family census again matches the committed entailment
ceiling exactly (injection 50, memory 176, access_control 8, ...).
Also: the Joern seam now runs in the CPG's own scratch directory, because
`joern --script` writes a `workspace/` beside the working directory and would
otherwise leave one inside the repository being analysed.
The taint query itself was corrected against the live engine: the sink is the
call's ARGUMENTS (data flows into arguments, not into the call node) and
reachableByFlows is called on the sink with the source. With that, the
three-hop chain our flat IR could not resolve is found:
request.args["cmd"] -> cmd -> "echo " + cmd -> full -> os.system(full)
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
2b973dc0db
commit
f59e0f5b5a
@@ -109,7 +109,7 @@ class JoernBackend:
|
||||
scratch = Path(tempfile.mkdtemp(prefix="ousast-cpg-"))
|
||||
cpg_path = scratch / "cpg.bin"
|
||||
command = [parse or "joern-parse", str(root), "--output", str(cpg_path)]
|
||||
completed = self._run(command, timeout=self.build_timeout)
|
||||
completed = self._run(command, timeout=self.build_timeout, cwd=scratch)
|
||||
if completed is None or completed.returncode != 0 or not cpg_path.is_file():
|
||||
detail = (completed.stderr or completed.stdout or "")[-400:] if completed is not None else "timeout"
|
||||
logger.warning("cpg build failed for %s: %s", root, detail)
|
||||
@@ -125,17 +125,19 @@ class JoernBackend:
|
||||
command = [shutil.which("joern") or "joern", "--script", str(script), "--param", f"cpgFile={cpg_path}"]
|
||||
for key, value in sorted(params.items()):
|
||||
command += ["--param", f"{key}={_render(value)}"]
|
||||
completed = self._run(command, timeout=self.query_timeout)
|
||||
# Joern writes a `workspace/` beside the working directory; run it inside the CPG's own scratch dir so
|
||||
# it can never land in the repository being analysed.
|
||||
completed = self._run(command, timeout=self.query_timeout, cwd=cpg_path.parent)
|
||||
if completed is None or completed.returncode != 0:
|
||||
detail = (completed.stderr or "")[-400:] if completed is not None else "timeout"
|
||||
logger.warning("cpg query %s failed: %s", query, detail)
|
||||
return None
|
||||
return extract_payload(completed.stdout or "")
|
||||
|
||||
def _run(self, command: list[str], *, timeout: int) -> subprocess.CompletedProcess[str] | None:
|
||||
def _run(self, command: list[str], *, timeout: int, cwd: Path | None = None) -> subprocess.CompletedProcess[str] | None:
|
||||
run = self.runner if self.runner is not None else subprocess.run
|
||||
try:
|
||||
return run(command, capture_output=True, text=True, timeout=timeout, check=False)
|
||||
return run(command, capture_output=True, text=True, timeout=timeout, check=False, cwd=str(cwd) if cwd else None)
|
||||
except (OSError, subprocess.SubprocessError):
|
||||
return None
|
||||
|
||||
|
||||
@@ -1,55 +1,60 @@
|
||||
// Taint reachability for one flow family (model-grounded-detection Req 6.1).
|
||||
//
|
||||
// The question this answers is the one our flat IR could not: does an untrusted source reach the labeled sink
|
||||
// *through resolved dataflow*, and is there a sanitizer on that path. Joern's `reachableByFlows` walks the
|
||||
// PDG, so a value bound three statements upstream — `q = "..." + request.args["x"]; execute(q)` — is found,
|
||||
// which is exactly the one-hop limit that held our measured entailment at 2.2%.
|
||||
// *through resolved dataflow*, and is there a sanitizer on that path. `reachableByFlows` walks the PDG, so a
|
||||
// value bound several statements upstream is found:
|
||||
//
|
||||
// Parameters (all comma-separated where plural):
|
||||
// cpgFile the built cpg.bin
|
||||
// sources source matchers, e.g. request.args,request.form
|
||||
// sinks sink call names, e.g. execute,os.system
|
||||
// sanitizers sanitizer call names; a flow through one of these is corroborated, not entailed
|
||||
// function optional: restrict sinks to this enclosing method
|
||||
// cmd = request.args["cmd"] request.args["cmd"] -> cmd -> "echo " + cmd -> full
|
||||
// full = "echo " + cmd
|
||||
// os.system(full)
|
||||
//
|
||||
// Output: a fenced JSON array of {sink, sinkLine, sinkMethod, source, sanitized, length}.
|
||||
// The fence exists because Joern prints a banner and a prompt around whatever a script emits.
|
||||
// That three-hop chain is exactly the one-hop limit which held our measured entailment at 2.2%.
|
||||
//
|
||||
// Two things here are easy to get backwards and were, first time round:
|
||||
// * the sink is the call's ARGUMENTS, not the call node -- data flows into arguments;
|
||||
// * `reachableByFlows` is called ON the sink WITH the source, not the other way about.
|
||||
//
|
||||
// Parameters (comma-separated where plural):
|
||||
// cpgFile, sources, sinks, sanitizers (optional), function (optional: restrict to this enclosing method)
|
||||
//
|
||||
// Output: a fenced JSON array of {sink, sinkLine, sinkMethod, source, sanitized, length}. The fence exists
|
||||
// because Joern prints a banner, pass logs and a prompt around whatever a script emits.
|
||||
|
||||
@main def exec(cpgFile: String, sources: String, sinks: String, sanitizers: String = "", function: String = "") = {
|
||||
importCpg(cpgFile)
|
||||
|
||||
def split(raw: String): List[String] =
|
||||
raw.split(",").map(_.trim).filter(_.nonEmpty).toList
|
||||
def split(raw: String): List[String] = raw.split(",").map(_.trim).filter(_.nonEmpty).toList
|
||||
|
||||
val sourcePatterns = split(sources)
|
||||
val sinkNames = split(sinks)
|
||||
val sanitizerNames = split(sanitizers)
|
||||
val sourcePatterns = split(sources)
|
||||
val sinkNames = split(sinks)
|
||||
val sanitizerNames = split(sanitizers)
|
||||
|
||||
// A source is any call or identifier whose code matches one of the source patterns. Matching on `code`
|
||||
// rather than on a method name keeps `request.args["x"]` and `req.body.name` both reachable without
|
||||
// per-framework special cases.
|
||||
def sourceNodes = cpg.call.filter(c => sourcePatterns.exists(p => c.code.contains(p))).l ++
|
||||
cpg.identifier.filter(i => sourcePatterns.exists(p => i.code.contains(p))).l
|
||||
// A source is any call whose code contains one of the patterns. Matching on `code` rather than a method
|
||||
// name keeps `request.args["x"]` (an indexAccess over a fieldAccess) and `req.body.name` both reachable
|
||||
// without a per-framework rule for each.
|
||||
def sourceNodes = cpg.call.filter(c => sourcePatterns.exists(p => c.code.contains(p)))
|
||||
|
||||
// A sink is a call whose name (or full code) names one of the family's sinks, optionally inside `function`.
|
||||
def sinkNodes = {
|
||||
val all = cpg.call.filter(c => sinkNames.exists(n => c.name == n || c.code.startsWith(n) || c.methodFullName.contains(n))).l
|
||||
// A sink call: matched by short name (`system`), by leading code (`os.system(...)`), or by resolved
|
||||
// full name (`os.py:<module>.system`), so both bare and dotted forms in the spec hit.
|
||||
def sinkCalls = {
|
||||
val all = cpg.call.filter(c =>
|
||||
sinkNames.exists(n => c.name == n || c.code.startsWith(n + "(") || c.code.startsWith(n) || c.methodFullName.contains(n))
|
||||
)
|
||||
if (function.isEmpty) all else all.filter(_.method.name == function)
|
||||
}
|
||||
|
||||
def isSanitized(flowCode: List[String]): Boolean =
|
||||
sanitizerNames.nonEmpty && flowCode.exists(code => sanitizerNames.exists(s => code.contains(s)))
|
||||
|
||||
val rows = sinkNodes.flatMap { sink =>
|
||||
val flows = sourceNodes.reachableByFlows(sink).l
|
||||
val rows = sinkCalls.l.flatMap { sink =>
|
||||
// Data flows into the arguments; asking the call node itself finds nothing.
|
||||
val flows = sink.argument.reachableByFlows(sourceNodes).l
|
||||
flows.map { flow =>
|
||||
val elements = flow.elements.map(_.code).l
|
||||
val sanitized = sanitizerNames.nonEmpty && elements.exists(code => sanitizerNames.exists(s => code.contains(s)))
|
||||
ujson.Obj(
|
||||
"sink" -> sink.code.take(200),
|
||||
"sinkLine" -> sink.lineNumber.getOrElse(-1).toString,
|
||||
"sinkMethod" -> sink.method.name,
|
||||
"source" -> elements.headOption.getOrElse("").take(200),
|
||||
"sanitized" -> isSanitized(elements),
|
||||
"sanitized" -> sanitized,
|
||||
"length" -> elements.size
|
||||
)
|
||||
}
|
||||
|
||||
@@ -18,7 +18,10 @@ from __future__ import annotations
|
||||
from collections.abc import Mapping, Sequence
|
||||
from dataclasses import dataclass, field
|
||||
from pathlib import Path
|
||||
from typing import Literal
|
||||
from typing import TYPE_CHECKING, Literal
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from .taxonomy import FamilyTaxonomy
|
||||
|
||||
CandidateKind = Literal["call", "bind", "operation", "config"]
|
||||
|
||||
@@ -255,7 +258,7 @@ class CandidateReport:
|
||||
}
|
||||
|
||||
|
||||
def ceiling(cases: Sequence[object], *, taxonomy: object, generator: str = "ir") -> CandidateReport:
|
||||
def ceiling(cases: Sequence[object], *, taxonomy: FamilyTaxonomy, generator: str = "ir") -> CandidateReport:
|
||||
"""Per family and per slice, the fraction of labeled functions a generator can put a candidate inside.
|
||||
|
||||
``generator`` is ``ir`` or ``ruleset``. The ruleset arm exists so the comparison that rejected it — 12.1%
|
||||
@@ -269,7 +272,7 @@ def ceiling(cases: Sequence[object], *, taxonomy: object, generator: str = "ir")
|
||||
gaps: list[tuple[str, str, str]] = []
|
||||
sites: dict[str, list[int]] = {}
|
||||
for case in cases:
|
||||
family = labeled_family(case)
|
||||
family = labeled_family(case, taxonomy)
|
||||
slice_name = str(getattr(case, "slice", "") or "")
|
||||
region_path = str(getattr(case, "relpath", "") or "")
|
||||
with tempfile.TemporaryDirectory(prefix="ousast-ceiling-") as scratch:
|
||||
@@ -329,17 +332,37 @@ def _distribution(counts: list[int]) -> dict[str, int]:
|
||||
}
|
||||
|
||||
|
||||
def labeled_family(case: object) -> str:
|
||||
"""The family a pair is labeled with, or ``unknown``.
|
||||
def labeled_family(case: object, taxonomy: FamilyTaxonomy | None = None) -> str:
|
||||
"""The family a pair belongs to, derived deterministically: declared label, else obligation, else mechanism,
|
||||
else CWE. ``unknown`` when none of those resolve.
|
||||
|
||||
The deleted ``learning/rounds.py`` fell back to the LLM classifier here. Nothing does now: an unlabeled
|
||||
pair is honestly ``unknown`` rather than guessed, which is the same discipline the evidence ladder applies
|
||||
to a claim the model cannot arbitrate.
|
||||
The deleted ``learning/classify.py`` had exactly this chain and only asked an LLM when every link failed —
|
||||
which, for a labeled corpus, was never. Group 1 first reduced this to the declared label alone, which
|
||||
silently emptied every family whose pairs are labeled by CWE rather than by name; the chain is restored
|
||||
here because it was never the noisy part.
|
||||
"""
|
||||
from .taxonomy import load_families
|
||||
|
||||
families: FamilyTaxonomy = taxonomy if taxonomy is not None else load_families()
|
||||
declared: list[str] = []
|
||||
derived: list[str] = []
|
||||
for row in getattr(case, "expected", ()) or ():
|
||||
family = getattr(row, "family", None)
|
||||
if family:
|
||||
return str(family)
|
||||
label = getattr(row, "family", None)
|
||||
if label and any(family.id == label for family in families.families):
|
||||
declared.append(str(label))
|
||||
continue
|
||||
if getattr(row, "obligation", None):
|
||||
derived.append("access_control")
|
||||
mechanism = getattr(row, "mechanism", None)
|
||||
by_mechanism = families.family_of_mechanism(str(mechanism)) if mechanism else None
|
||||
if by_mechanism is not None and by_mechanism.id != "unknown":
|
||||
derived.append(by_mechanism.id)
|
||||
continue
|
||||
by_cwe = families.family_of_cwe(str(getattr(row, "cwe", "") or ""))
|
||||
if by_cwe is not None and by_cwe.id != "unknown":
|
||||
derived.append(by_cwe.id)
|
||||
for family in declared or derived:
|
||||
return family
|
||||
return "unknown"
|
||||
|
||||
|
||||
|
||||
@@ -76,13 +76,28 @@ def classify_guard_text(text: str) -> str:
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class TaintSpec:
|
||||
"""What a flow family's source→sink−sanitizer query matches on, for one language."""
|
||||
"""What a flow family's source-to-sink-minus-sanitizer query matches on, for one language.
|
||||
|
||||
``sanitizers`` holds only *cleansing calls* -- a node on the flow path that makes the value safe, such as
|
||||
``ast.literal_eval``. It deliberately excludes every fact that carries a **shape qualifier**, because
|
||||
those describe a safe *form of the sink call*, not a call that cleans a value:
|
||||
|
||||
parameterized ``execute(sql, params)`` binds rather than interpolates
|
||||
literal_format_arg ``printf("literal", x)`` has a constant format string
|
||||
|
||||
Flattening either kind into the sanitizer list is actively wrong: the sink becomes its own sanitizer, so
|
||||
every flow through it reports as already-clean and nothing is ever entailed. Both were present in the
|
||||
shipped facts (`execute` for Python, the whole printf family for C) and both surfaced on the first live
|
||||
Joern run. The names are kept in ``safe_shape_sinks`` so the shape test can be modelled properly in task
|
||||
4.2 rather than silently mis-modelled here.
|
||||
"""
|
||||
|
||||
family: str
|
||||
language: str
|
||||
sources: tuple[str, ...]
|
||||
sinks: tuple[str, ...]
|
||||
sanitizers: tuple[str, ...]
|
||||
safe_shape_sinks: tuple[str, ...] = ()
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
@@ -113,7 +128,12 @@ def taint_specs(
|
||||
scoped = (facts if facts is not None else load_facts()).for_language(language)
|
||||
families = taxonomy if taxonomy is not None else load_families()
|
||||
sources = tuple(sorted({pattern for fact in scoped.sources for pattern in fact.patterns}))
|
||||
sanitizers = tuple(sorted({call for fact in scoped.sanitizers for call in fact.calls}))
|
||||
# A shape qualifier means "this sink is safe in this form", never "this call cleans the value".
|
||||
def _is_shape(fact: object) -> bool:
|
||||
return bool(getattr(fact, "parameterized", False)) or getattr(fact, "literal_format_arg", None) is not None
|
||||
|
||||
sanitizers = tuple(sorted({call for fact in scoped.sanitizers if not _is_shape(fact) for call in fact.calls}))
|
||||
safe_shapes = tuple(sorted({call for fact in scoped.sanitizers if _is_shape(fact) for call in fact.calls}))
|
||||
by_family: dict[str, set[str]] = {}
|
||||
for sink in scoped.sinks:
|
||||
family = families.family_of_cwe(sink.cwe)
|
||||
@@ -121,7 +141,14 @@ def taint_specs(
|
||||
continue
|
||||
by_family.setdefault(family.id, set()).update(sink.calls)
|
||||
return {
|
||||
family_id: TaintSpec(family=family_id, language=language, sources=sources, sinks=tuple(sorted(calls)), sanitizers=sanitizers)
|
||||
family_id: TaintSpec(
|
||||
family=family_id,
|
||||
language=language,
|
||||
sources=sources,
|
||||
sinks=tuple(sorted(calls)),
|
||||
sanitizers=sanitizers,
|
||||
safe_shape_sinks=safe_shapes,
|
||||
)
|
||||
for family_id, calls in sorted(by_family.items())
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user