mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-18 12:39:28 +00:00
fix(scan): a catch-all function's parameters are not attacker input; field reads need the same field
Two classes the traced PMPro survivors named after 1f91c0b:
- The mapper records every named PHP function as an entry of kind
`function` (3,507 of PMPro's 4,412), and every entry's parameters were
sources, so internal helpers became injections: sendCancelAdminEmail's
$user, a gateway hook callback's $user_id/$morder fired by do_action.
Regions now carry the entry kind; parameters are input only for request
entries (route, parser, privileged surface). Call depth is unchanged, so
such a region is still followed into its callees. JavaScript and Python
emit no `function` entries.
- The object rule is general: a field read continues a flow through the
bare object only if the taint came through the SAME field, or was
assigned to the variable or passed in as a parameter. It now also cuts
$this->Gateway -> this -> $this->user_id (memberorder.php:705).
plausible() memoises the sink test per node and skips the object walk for
paths with no field access (the filter had cost one outlier 101 s -> 160 s).
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
1f91c0b089
commit
da0ee576f8
@@ -302,12 +302,6 @@
|
||||
def fieldOf(c: io.shiftleft.codepropertygraph.generated.nodes.Call): String =
|
||||
c.argument.l.find(_.argumentIndex == 2).map(_.code.trim).getOrElse("")
|
||||
|
||||
def writtenField(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean =
|
||||
c.argumentIndex == 1 && (c.astParent match {
|
||||
case parent: io.shiftleft.codepropertygraph.generated.nodes.Call => parent.name == "<operator>.assignment"
|
||||
case _ => false
|
||||
})
|
||||
|
||||
def assignedTo(id: io.shiftleft.codepropertygraph.generated.nodes.Identifier): Boolean =
|
||||
id.argumentIndex == 1 && (id.astParent match {
|
||||
case parent: io.shiftleft.codepropertygraph.generated.nodes.Call => parent.name == "<operator>.assignment"
|
||||
@@ -324,8 +318,10 @@
|
||||
if (j < 0) false // the object itself is where the path starts: it IS the tainted value
|
||||
else
|
||||
elements(j) match {
|
||||
case prior: io.shiftleft.codepropertygraph.generated.nodes.Call if prior.name == "<operator>.fieldAccess" =>
|
||||
writtenField(prior) && fieldOf(prior) != fieldOf(read)
|
||||
// Through the SAME field -- written then read back, or read and read again -- is field-sensitive.
|
||||
case prior: io.shiftleft.codepropertygraph.generated.nodes.Call
|
||||
if prior.name == "<operator>.fieldAccess" && fieldOf(prior) == fieldOf(read) =>
|
||||
false
|
||||
case _ =>
|
||||
// How did the value reach the object? Legitimately only by being ASSIGNED to it
|
||||
// (`$data = json_decode($input)`) or passed in as a parameter. An object merely used beside
|
||||
@@ -340,9 +336,21 @@
|
||||
}
|
||||
}
|
||||
|
||||
// Per element, not per path: the same nodes recur across thousands of paths of one sink, and walking each
|
||||
// path's calls afresh cost one outlier request 101 s -> 160 s.
|
||||
val familySinkMemo = scala.collection.mutable.Map.empty[Long, Boolean]
|
||||
|
||||
def plausible(flow: io.joern.dataflowengineoss.language.Path): Boolean = {
|
||||
val elements = flow.elements.l
|
||||
!elements.dropRight(1).exists(familySinkCall) && !absorbedByObject(elements)
|
||||
val throughSink = elements.dropRight(1).exists {
|
||||
case c: io.shiftleft.codepropertygraph.generated.nodes.Call if !c.name.startsWith("<operator") =>
|
||||
familySinkMemo.getOrElseUpdate(c.id, familySinkCall(c))
|
||||
case _ => false
|
||||
}
|
||||
!throughSink && !(elements.exists {
|
||||
case c: io.shiftleft.codepropertygraph.generated.nodes.Call => c.name == "<operator>.fieldAccess"
|
||||
case _ => false
|
||||
} && absorbedByObject(elements))
|
||||
}
|
||||
|
||||
// Framework sources are untrusted wherever they appear. Parameters are untrusted only where the caller has
|
||||
|
||||
Reference in New Issue
Block a user