marius/Open_Ultra_SAST
1
0
Fork 0
mirror of https://github.com/norandom/OpenUltraSAST.git synced 2026-10-16 03:49:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(taint): project-local sink wrappers are sinks where they are called

Browse Source 
Real code rarely calls the driver where it builds the query. YesWiki builds its
INSERT in `FormManager::create($data)` and runs it through
`$this->dbService->query($query)`, whose body is the `mysqli_query` the facts name.
The only sink the facts knew sat in a helper that no request reached directly.

A method whose parameter feeds a family sink is now a sink for that argument,
reported at the call site. The summary is structural, to stay cheap and
deterministic. It is not a flow query per parameter, which would repeat at every
engine start of a portioned scan. The parameter must reach the sink call's
argument directly or through at most two local assignments, with at least one
occurrence outside every cleansing call. It iterates up to three levels, so a
wrapper of a wrapper counts.

Calls resolve by name, because an injected receiver has no type the frontend can
see. A name therefore counts only if every project method with that name is a
wrapper. At a wrapper call, only the summarised arguments are asked.

Two Joern-gated tests cover:
- a request value into a second-level wrapper, reported at the controller's call;
- direct use, a literal, and an `intval`-casting helper, which are not wrappers;
- a harmless same-named method, which disqualifies the name.
Both fail with their part removed.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
Marius Ciepluch
2026-09-26 07:55:24 +02:00
co-authored by Claude Opus 5.5
parent 64ef488ce4
commit 54c9dfcd44
2 changed files with 146 additions and 2 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/openultrasast/cpg/queries/taint.sc
+93 -2
View File
@@ -72,6 +72,7 @@
// Per-family answers that do not depend on the region, kept across the whole batch.
val familyInRepoMemo = scala.collection.mutable.Map.empty[String, Boolean]
val sinkCandidatesMemo = scala.collection.mutable.Map.empty[String, List[io.shiftleft.codepropertygraph.generated.nodes.Call]]
val wrapperMemo = scala.collection.mutable.Map.empty[String, Map[String, Set[Int]]]
val reachableMemo = scala.collection.mutable.Map.empty[(String, String, Int), Set[String]]
val methodSourceMemo = scala.collection.mutable.Map.empty[String, Boolean]
val fedFieldsMemo = scala.collection.mutable.Map.empty[String, Map[String, String]]
@@ -1104,9 +1105,99 @@
// for each of 2,487 requests was a large share of what made a request cost seconds -- in evidence mode,
// which asks for no dataflow at all, it alone took a query past an hour. Scanned once per sink list and
// kept for the batch; the per-request work is the scope filter over that list.
// ---- project-local sink WRAPPERS ------------------------------------------------------------------------
//
// Real code rarely calls the driver where it builds the query. YesWiki builds its INSERT in
// `FormManager::create($data)` and runs it through `$this->dbService->query($query)`, whose body is the
// `mysqli_query` the facts name -- so the one sink the facts knew sat in a helper no request ever reached
// directly, and the injectable value was never asked about where it entered. A method whose parameter
// feeds a family sink is itself a sink for that argument, reported where it is CALLED.
//
// Summarised STRUCTURALLY, not by dataflow: the parameter reaches the sink call's argument directly or
// through at most two local assignments, and at least one occurrence is outside every cleansing call. A
// flow query per parameter per method, repeated at every engine start of a portioned scan, is what this
// project's scale walls were made of. Up to three levels (a wrapper of a wrapper). Calls resolve by NAME,
// because an injected receiver (`$this->dbService`) has no type the frontend can see -- so a name counts
// only if every project method so named is a wrapper; one harmless `query()` elsewhere disqualifies it.
def underCleansing(node: io.shiftleft.codepropertygraph.generated.nodes.AstNode, root: io.shiftleft.codepropertygraph.generated.nodes.AstNode): Boolean = {
var current: Option[io.shiftleft.codepropertygraph.generated.nodes.AstNode] = Some(node)
var found = false
while (current.isDefined && !found && current.get.id != root.id) {
if (current.get.id != node.id && sanitizesHere(current.get, strictNames)) found = true
current = current.get._astIn.nextOption() match {
case Some(parent: io.shiftleft.codepropertygraph.generated.nodes.AstNode) => Some(parent)
case _ => None
}
}
found
}
def feeds(
method: io.shiftleft.codepropertygraph.generated.nodes.Method,
expression: io.shiftleft.codepropertygraph.generated.nodes.AstNode,
name: String,
depth: Int
): Boolean =
depth <= 2 && {
val identifiers = expression.ast.isIdentifier.l
identifiers.exists(i => i.name == name && !underCleansing(i, expression)) ||
identifiers.map(_.name).distinct.filter(_ != name).exists { local =>
method.ast.isCall.nameExact(ASSIGNMENT).l.exists { assignment =>
val args = assignment.argument.l
args.find(_.argumentIndex == 1).exists {
case target: io.shiftleft.codepropertygraph.generated.nodes.Identifier => target.name == local
case _ => false
} && args.find(_.argumentIndex == 2).exists(value => feeds(method, value, name, depth + 1))
}
}
}
lazy val wrappers: Map[String, Set[Int]] =
wrapperMemo.getOrElseUpdate(
sinksS + "|" + sanitizersS + "|" + quotedSanitizersS, {
var known = Map.empty[String, Set[Int]]
var wrapperMethods = Set.empty[String]
var frontier = sinkCandidatesMemo.getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
var level = 0
var growing = true
while (level < 3 && growing && frontier.nonEmpty) {
val found = frontier.groupBy(_.method).toList.flatMap { case (method, calls) =>
if (method.isExternal || method.name.startsWith("<")) Nil
else
method.parameter.l
.filter(p => p.index >= 1 && !objectBases.contains(p.name) && p.name != "self" && p.name != "this")
.filter(p => calls.exists(c => c.argument.argumentIndexGt(0).l.exists(a => feeds(method, a, p.name, 0))))
.map(p => (method, p.index))
}
val added = found.filterNot { case (m, index) => known.get(m.name).exists(_.contains(index)) }
growing = added.nonEmpty
added.foreach { case (m, index) =>
known = known.updated(m.name, known.getOrElse(m.name, Set.empty[Int]) + index)
wrapperMethods += m.fullName
}
val names = added.map(_._1.name).toSet
frontier = if (names.isEmpty) Nil else cpg.call.filter(c => !c.name.startsWith("<operator") && names.contains(c.name)).l
level += 1
}
known.filter { case (name, _) => cpg.method.nameExact(name).filterNot(_.isExternal).l.forall(m => wrapperMethods.contains(m.fullName)) }
}
)
def wrapperCall(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean =
!c.name.startsWith("<operator") && wrappers.contains(c.name) && !sinkNames.exists(n => sinkMatches(c, n))
// The arguments a flow must arrive through: every real argument of a fact sink, only the summarised ones of
// a wrapper.
def sinkTargets(c: io.shiftleft.codepropertygraph.generated.nodes.Call) =
if (wrapperCall(c)) c.argument.l.filter(a => wrappers(c.name).contains(a.argumentIndex))
else c.argument.argumentIndexGt(0).l
def sinkCalls =
sinkCandidatesMemo
.getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
.getOrElseUpdate("wrapped|" + sinksS + "|" + sanitizersS + "|" + quotedSanitizersS, {
val facts = sinkCandidatesMemo.getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l)
if (wrappers.isEmpty) facts else facts ++ cpg.call.filter(wrapperCall).l
})
.filter(c => inScope(c.method))
// ---- stage one of the two-stage join WITHOUT dataflow, for evidence mode (flow-aware-ranking, phase 2) --
@@ -1357,7 +1448,7 @@
// no data at all, yet `res` is reachable from the request in any handler that captures it, so the flow
// query answered yes about a call that receives nothing. Measured on NodeGoat, where `res.write(body)` is a
// real finding and `res.end()` two lines below it was reported identically.
val flows = if (hasSources) sink.argument.argumentIndexGt(0).reachableByFlows(sourceNodes).l.filter(plausible) else Nil
val flows = if (hasSources) sinkTargets(sink).reachableByFlows(sourceNodes).l.filter(plausible) else Nil
// The SHAPE of the sink call, which is what distinguishes a fix from a bug when the fix is a safe form
// rather than a sanitizing call: `execute(sql, params)` binds where `execute(sql + x)` interpolates, and
tests/test_cpg_queries_compile.py
+53
View File
@@ -1417,3 +1417,56 @@ def test_an_allowlist_check_guards_its_branch_and_its_ternary_arm(tmp_path: Path
payload = _taint_rows(tmp_path, "php2cpg", {"app.php": app}, {n: {**common, "function": n} for n in names})
reported = {n for n in names if _unsanitized(payload, n)}
assert reported == {"wrong_arm", "else_branch", "stored_in_else", "haystack"}, {n: payload[n] for n in names}
_WRAPPED_PHP = (
"<?php\n"
"class DbService { public $link;\n"
" function query( $query ) { return mysqli_query( $this->link, $query ); }\n"
" function number( $n ) { return mysqli_query( $this->link, 'SELECT ' . intval( $n ) ); } }\n"
"class FormManager { public $dbService;\n"
" function create( $data ) { $query = 'INSERT INTO t VALUES (' . $data['id'] . ')';\n"
" return $this->dbService->query( $query ); } }\n"
"class FormController { public $formManager; public $dbService;\n"
" function displayAll() { $value = $_POST['form']; return $this->formManager->create( $value ); }\n"
" function direct() { return $this->dbService->query( 'SELECT ' . $_POST['q'] ); }\n"
" function literal() { return $this->dbService->query( 'SELECT 1' ); }\n"
" function cast() { return $this->dbService->number( $_POST['n'] ); } }\n"
)
def _wrapped_requests(spec, names):
common = {
"sources": ",".join(spec.sources),
"sinks": ",".join(spec.sinks),
"sanitizers": ",".join(spec.sanitizers),
"quotedSanitizers": ",".join(spec.quoted_sanitizers),
"file": "app.php",
}
return {n: {**common, "function": n} for n in names}
@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
def test_a_project_sink_wrapper_is_a_sink_where_it_is_called(tmp_path: Path) -> None:
"""YesWiki runs its INSERT through `$this->dbService->query($query)`, whose body is the `mysqli_query` the facts
name, and builds it in `FormManager::create($data)`. Neither receiver has a type the frontend can see. The
request value is reported where it enters a wrapper -- two levels up -- and not where it cannot."""
from openultrasast.model.specs import taint_specs
spec = taint_specs(language="php")["injection"]
names = ("displayAll", "direct", "literal", "cast")
payload = _taint_rows(tmp_path, "php2cpg", {"app.php": _WRAPPED_PHP}, _wrapped_requests(spec, names))
reported = {n: [r for r in _unsanitized(payload, n) if r.get("sinkMethod") == n] for n in names}
assert {n for n in names if reported[n]} == {"displayAll", "direct"}, {n: payload[n] for n in names}
assert "create" in str(reported["displayAll"][0]["sink"]), reported["displayAll"]
@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
def test_a_wrapper_name_shared_with_a_harmless_method_is_not_a_sink(tmp_path: Path) -> None:
"""Calls resolve by name, so a name is a sink only if every project method so named is a wrapper."""
from openultrasast.model.specs import taint_specs
spec = taint_specs(language="php")["injection"]
code = _WRAPPED_PHP + "class Cache { function query( $key ) { return strlen( $key ); } }\n"
payload = _taint_rows(tmp_path, "php2cpg", {"app.php": code}, _wrapped_requests(spec, ("direct",)))
assert not [r for r in _unsanitized(payload, "direct") if r.get("sinkMethod") == "direct"], payload["direct"]