mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-17 03:59:28 +00:00
Revert "feat(taint): project-local sink wrappers are sinks where they are called"
This reverts commit 54c9dfc. The recall-pin check (p16) showed a large precision
and completion regression, although every recall target survived:
- MW WP Form: 3 -> 60 findings. Form-field renderers became sinks.
- WP Statistics: 16 -> 40.
- PMPro: 16 -> 65, 20 of them inside bundled Braintree/Stripe code. Completion fell
to 390 of 1189 questions, because wrappers made many more regions look
sink-bearing.
- YesWiki: completion fell from 2798/2807 to 216/2807.
The structural summary was too loose. Any identifier in a sink argument counted as
fed by a parameter if any assignment anywhere in the method mentioned it, with no
order or dominance. It also ignored vendored library code and cascaded three
levels by name. A redesign needs dataflow-confirmed, level-limited summaries that
leave the question count alone.
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
54c9dfcd44
commit
c7e3a05ba0
@@ -1417,56 +1417,3 @@ 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"]
|
||||
|
||||
Reference in New Issue
Block a user