mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(taint): a guard validates only the value it tests
p14 showed PMPro's functions.php:906 discharged by `elseif (in_array('E', $levels))`.
That checks whether a constant is in `$levels`; it says nothing about `$levels`. The
guard counted a carried variable in any argument. Only argument 1, the tested value
(`in_array`'s needle, the URL given to `url_has_allowed_host_and_scheme`), is
validated now. The allowlist test gains the haystack shape as a reported control.
The test fails when any argument counts.
The other p14 removals were right: WP Statistics export_data x2
(`if (!in_array($table, $allow_tbl)) $table = false;`) and PMPro memberorder.php:1633.
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
69d4a8626f
commit
63460f8af4
@@ -710,8 +710,11 @@
|
|||||||
def guardsIn(root: io.shiftleft.codepropertygraph.generated.nodes.AstNode, names: Set[String]): List[io.shiftleft.codepropertygraph.generated.nodes.Call] =
|
def guardsIn(root: io.shiftleft.codepropertygraph.generated.nodes.AstNode, names: Set[String]): List[io.shiftleft.codepropertygraph.generated.nodes.Call] =
|
||||||
if (guardNames.isEmpty) Nil
|
if (guardNames.isEmpty) Nil
|
||||||
else
|
else
|
||||||
|
// Only the TESTED value is validated -- the first argument: `in_array`'s needle, the URL a redirect check is
|
||||||
|
// given. PMPro's `in_array( 'E', $levels )` asks whether a constant is in `$levels`; with any argument
|
||||||
|
// counted, it discharged a flow through `$levels` itself.
|
||||||
root.ast.isCall.l.filter(g => guardNames.contains(g.name)).filter { g =>
|
root.ast.isCall.l.filter(g => guardNames.contains(g.name)).filter { g =>
|
||||||
g.argument.l.exists(_.ast.exists {
|
g.argument.l.filter(_.argumentIndex == 1).exists(_.ast.exists {
|
||||||
case i: io.shiftleft.codepropertygraph.generated.nodes.Identifier => names(i.name) || names(i.code.trim)
|
case i: io.shiftleft.codepropertygraph.generated.nodes.Identifier => names(i.name) || names(i.code.trim)
|
||||||
case c: io.shiftleft.codepropertygraph.generated.nodes.Call if c.name == FIELD_ACCESS => names(c.code.trim)
|
case c: io.shiftleft.codepropertygraph.generated.nodes.Call if c.name == FIELD_ACCESS => names(c.code.trim)
|
||||||
case _ => false
|
case _ => false
|
||||||
|
|||||||
@@ -1400,8 +1400,11 @@ def test_an_allowlist_check_guards_its_branch_and_its_ternary_arm(tmp_path: Path
|
|||||||
' else { $o = " ORDER BY u.{$s} "; }\n'
|
' else { $o = " ORDER BY u.{$s} "; }\n'
|
||||||
" $o = apply_filters( 'sort', $o, $s );\n"
|
" $o = apply_filters( 'sort', $o, $s );\n"
|
||||||
' return $wpdb->query( "SELECT 1 FROM t {$o}" ); }\n'
|
' return $wpdb->query( "SELECT 1 FROM t {$o}" ); }\n'
|
||||||
|
"function haystack() { global $wpdb; $l = $_POST['l'];\n"
|
||||||
|
" if ( in_array( 'E', $l, true ) ) { return $wpdb->query( \"SELECT 1 FROM t WHERE u = {$l[0]}\" ); }\n"
|
||||||
|
" return null; }\n"
|
||||||
)
|
)
|
||||||
names = ("ternary", "wrong_arm", "branch", "else_branch", "stored_in_branch", "stored_in_else")
|
names = ("ternary", "wrong_arm", "branch", "else_branch", "stored_in_branch", "stored_in_else", "haystack")
|
||||||
common = {
|
common = {
|
||||||
"sources": ",".join(spec.sources),
|
"sources": ",".join(spec.sources),
|
||||||
"sinks": ",".join(spec.sinks),
|
"sinks": ",".join(spec.sinks),
|
||||||
@@ -1413,4 +1416,4 @@ 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})
|
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)}
|
reported = {n for n in names if _unsanitized(payload, n)}
|
||||||
assert reported == {"wrong_arm", "else_branch", "stored_in_else"}, {n: payload[n] for n in names}
|
assert reported == {"wrong_arm", "else_branch", "stored_in_else", "haystack"}, {n: payload[n] for n in names}
|
||||||
|
|||||||
Reference in New Issue
Block a user