mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 11:49:28 +00:00
c64e37edbdb5c626096d658e143bcd15e24677cd
31
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Marius CiepluchandClaude Opus 5.5
|
d14fd5e960 |
Revert "feat(taint): project sink wrappers, one level and confirmed by dataflow"
This reverts commit
|
||
|
Marius CiepluchandClaude Opus 5.5
|
cc885f025c |
feat(taint): project sink wrappers, one level and confirmed by dataflow
This is the second attempt at the YesWiki shape: SQL run through
`$this->dbService->query($query)`, whose body is the `mysqli_query` the facts name,
through a receiver the frontend cannot type. The first attempt was reverted in
|
||
|
Marius CiepluchandClaude Opus 5.5
|
c7e3a05ba0 |
Revert "feat(taint): project-local sink wrappers are sinks where they are called"
This reverts commit
|
||
|
Marius CiepluchandClaude Opus 5.5
|
54c9dfcd44 |
feat(taint): project-local sink wrappers are sinks where they are called
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 |
||
|
Marius CiepluchandClaude Opus 5.5
|
63460f8af4 |
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
|
||
|
Marius CiepluchandClaude Opus 5.5
|
69d4a8626f |
fix(taint): a value stored while its guard held stays guarded
Ultimate Member's fixed pin kept one alert. The fix writes
`$this->sql_order = " ORDER BY u.{$sortby} "` inside
`elseif (in_array($sortby, $allowed, true))`, and after the if/elseif chain it
passes the field through `apply_filters(...)` into a second assignment. The guard
was judged only at the flow's final node, which is outside the branch.
A guard is now also judged at every point where the path stores the value (an
assignment target). A value stored while the check held carries the check forward,
because the rest of the path derives from that store. A mere read inside a branch
is not a candidate: the engine lists uses in branches the flow does not depend on.
The Joern-gated test gains the shape and its control: the same store in the
unguarded else branch, through the same filter, is still reported. The test fails
when only the end is judged.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
03f2ce7162 |
feat(taint): guards -- a validator or allowlist check makes a value safe where it held
Three of the four fixed-side alerts left in the independent population had one cause: the fix was a check, and checks were not modelled. - wger's fix is `url_has_allowed_host_and_scheme`: `if not ...(next_url): next_url = reverse(..)` and `if next_url and ...(next_url): return redirect`. - Ultimate Member's fix is an allowlist: `in_array(strtoupper($order), [...], true) ? $order : 'ASC'` and `elseif (in_array($sortby, $allowed, true))`. (The fourth, Signal K, is `assertAllowedHost`, a project-local function. It is not covered.) Sanitizer facts can now say `guard = true`. A guard is a check, never a cleansing call on a path. A flow is discharged only when a guard tests a variable the path carries, and the flow is in one of these places: - inside the branch whose condition requires the check. The guard may be combined with `&&` only; - past a failure test, a single `!` combined with `||` only, whose branch exits (return, exit, die, wp_die, throw) or overwrites the value, and whose condition dominates the flow's end; - through a ternary whose condition requires the check, where the other arm cannot carry the value. The engine can enter through the condition's own copy of the value, which never becomes the result. Facts: `in_array` (PHP) and Django's `url_has_allowed_host_and_scheme` / `is_safe_url` (Python). A row now consults guards even when a request lists no sanitizers. Two Joern-gated tests cover the discharged shapes and, as controls, an unguarded redirect, a guard on a different variable, a failure that is only logged, the unguarded else branch, and the ternary arm the check does not select. Both tests fail without the guard. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
3358631448 |
fix(scan,taint): TypeScript's limit is owned by TypeScript; a source pattern is a token
Two defects found by the cause-B rescan of the independent population (exploratory, a retune after protocol-v1): - `typescript_property_support_unvalidated` still reached the whole repository. YesWiki is a PHP application that ships a few `.ts` files, and all 1,128 of its PHP questions were unresolved. The limit is now owned by the TypeScript partition. A TypeScript project (Mockoon, Signal K) is unchanged. - `isSourceCall`, and the method-level "reads a source" heuristics, matched source patterns as substrings. Flask's `request.get` therefore claimed Django's `request.get_host()`. wger's fix passes that host to `url_has_allowed_host_and_scheme`, and both fixed functions were reported as open redirects from it. Patterns are now matched as bounded tokens, like sinks and sanitizers. wger's vulnerable-pin detections rest on `request.GET`, which still matches. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
949977c7a6 |
fix(taint): an SQL escape protects only a quoted value; splice the engine's object detours
`esc_sql` and the mysql/pg `*_escape_string` functions only backslash quotes. A value
they escape is safe inside a quoted literal and nowhere else. Ultimate Member
CVE-2024-1071 is `$sortby = esc_sql(...)` built into `" ORDER BY u.{$sortby} "`, and
it read as sanitized. Sanitizer facts can now say `quoted_only`. The escape keeps its
credit unless the first string the value is built into (concat, interpolation,
addition, f-string) places it after literal text that opens no quote. With no string
built downstream, the credit stays as before. pg_escape_literal adds its own quotes
and stays a full cleanser.
Two further defects hid the same CVE:
- The engine is field-insensitive on the object. Its one path from `$this->sql_order`
went `$this` -> `$this->having` -> `esc_sql($this->having)` -> ... -> the query.
absorbedByObject cut it, and it credited an unrelated field's escape. When a path
leaves a field through the object for another field and reads the first field
again, the detour is spliced out. The later read may be on the path or inside the
sink argument where the path ends.
- The field join judged a field clean if ANY sanitizer name was on its path. It now
uses the same quote-aware check as a row. This matters because the engine's own
path to the query changed between runs: 2 of 4 identical runs chose a path via
`$wpdb->prepare`. The field join is the deterministic route.
Exploratory (retune after protocol-v1): the Ultimate Member vulnerable pin is detected
at class-member-directory-meta.php:936. The fixed pin still alerts at :945. Its fix is
an allowlist (`in_array(...) ? $v : 'ASC'`, and an `in_array` guard), which is not
modelled yet.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
562b670a43 |
feat(rules): Django request sources; a destination with a fixed origin is not untrusted
python.toml's sources were Flask-only, so no flow could start anywhere on a Django
project. OpenCVE's SSRF reads its URL from `form.cleaned_data`, and the scan answered
the sink file and found nothing. Added: request.GET/POST/body/COOKIES/FILES/META,
DRF's query_params, and cleaned_data.
With those sources known, OpenCVE reported three redirects that cannot leave the
site: `reverse("cves") + "?" + request.GET.urlencode()` (twice) and
`reverse(route, kwargs=...)`. Destination sink facts can now set
`prefix_fixes_origin` and `origin_anchors`. A path is cut when it enters a
concatenation, f-string or `+=` through anything other than its leftmost operand,
and that operand fixes the origin. Origin-fixing operands:
- a literal `/x`, `?` or `#`;
- a literal of the form scheme://host/;
- an anchor call;
- a variable whose every assignment fixes the origin.
A path is also cut when it passes through an anchor. `"/" + x` (//evil) and
`"https://" + host` stay reported. A Joern-gated test covers all seven shapes.
Exploratory (retune after protocol-v1): OpenCVE vulnerable and fixed pins are both
0 findings, down from 2 and 3. The SSRF itself is still missed, because the flow
crosses a registry dispatch (`REGISTRY.get(t)(extras).run()`) that the engine does
not follow.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
ec3d1457fb |
fix(rules): Node vm is an eval sink; a package name is not a sink token
mongo-express CVE-2019-10758 passes the request body to vm.runInNewContext. No
injection sink named that call, so the flow was answered and missed. runInNewContext,
runInContext, runInThisContext and compileFunction are now CWE-95 sinks.
Once the file's questions completed, the fixed pin showed a second defect. Every call
on `require('mongodb-query-parser')` has that package name as its full name. The
bounded-token match treats `-` as a boundary, so the name matched the SQL sink
`query`. In a full name, `-` belongs to a package name, so it no longer separates
tokens.
Exploratory (retune after protocol-v1): mongo-express vulnerable pin now detected at
lib/bson.js:60.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
d5277c60d0 |
fix(taint): a field the method itself just overwrote is not object state
The largest remaining class in the third PMPro sample (2 of 4 false positives): every MemberOrder method builds its statement in $this->sqlQuery and runs it at once. One method writes request data there, the field join keyed the field file-wide, and deleteMe() -- which had just set it from the internal $this->id -- and updateTimestamp() (no caller in the plugin) were reported as SQL injections. A field read is now a field-join source only when it reads state another method left: not the target of a write, and not a read that the method's own write to the same field dominates. If that own write carries request data, the ordinary flow from the source still finds it. A real-Joern control covers all three: the method whose write carries $_POST (found), the method that overwrote it with an internal id (quiet), and a method reading what another left (found). It fails on the old query. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
6c03b4a38c |
fix(taint): a sanitizer the value passes through counts; only an access is a source
Found on PMPro's re-added class-pmpro-levels.php:87: every REST handler casts the level id, `isset($p["id"]) ? intval($p["id"]) : null`, and the flow was reported unsanitized. Traced, the engine steps from intval's argument straight to the conditional, so `intval` is never a path element and the element-wise sanitizer check never saw it. - sanitizedBetween: where a path element contains the previous one, the syntax between them is what the value was wrapped in; a sanitizer there sanitizes the flow. A sanitizer elsewhere in the expression (the other branch of a conditional) does not count. Used by the sink query and the hook join; the field join already matched sanitizer text. - The test exposed the conditional itself as a source: isSourceCall matched any operator by its full text. Of the operators only an access (field/index) names its own value; a conditional or concatenation built from `$_GET` is not a source. A real-Joern control covers the cast inside a conditional (sanitized) and a sanitizer in the other branch (not); 20 query tests pass. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
d57966a6b6 |
fix(taint): the field join links methods only through the object itself
profile.php:424 survived the hook-argument fix through the field join: in another function of the same file, `$user = new stdClass; $user->ID = $_POST['user_id']`, and the join keys a field by its TEXT across the whole file, so the WP_User parameter's `$user->ID` became a field source. The join exists to carry object state between methods ($this->attachments written in the constructor, read in _delete_files). That reading of a field's text holds only when the base IS the object ($this, this, self). Any other base is a variable, matched within its own method only, where the engine's dataflow already sees it. A real-Joern control covers both (a local variable's field in another function stays quiet; $this state across methods is traced) and fails on the old query. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
7b8ff3cf44 |
fix(taint): a filter returns its value argument; a source is the access, not a call around it
The apply_filters class from the second PMPro adjudication (2 of 3
remaining false positives): `apply_filters('pmpro_random_code', $code,
$this)` returned whatever $this carried, and a report's `$type` reached SQL
only as an extra filter argument. Two causes, both fixed generally:
- The dispatch fact now says what an apply call RETURNS: `returns` names
the calls whose result is the dispatched value and `value_arg` its
position (WordPress: apply_filters/apply_filters_ref_array return
argument 2; do_action returns nothing). A path through such a call must
ENTER by that argument -- the first of the run of the call's arguments
before it, because the engine hops between sibling arguments
($_GET["c"] -> md5("x") -> apply_filters). Loaded and validated as data;
the query knows no framework.
- A call was a source when the pattern appeared anywhere in its text, so
`apply_filters(..., $_GET["c"])` and `sanitize_text_field($_POST[..])`
were sources themselves -- the "a token matches the thing it names"
invariant. A call is now a source when the pattern names its callee,
receiver or the access it is; the inner access stays a source.
Controls: dispatch facts load/validate; a real-Joern case with request
data as filter context (quiet) and as the filtered value (found). 1,453
host and 18 query tests pass.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
1f91c0b089 |
fix(taint): drop paths through a query's result or through a whole object
Traced element by element (new opt-in `trace` on taint requests), every
false PMPro SQL injection in the adjudicated sample took one of two steps
no real injection needs, both over-approximations of Joern's dataflow:
A. Through a query: `$id = $wpdb->get_var("...'" . $token . "'")` is the
injection; the engine carried $token on through the query's RESULT
into later queries (getMemberOrderByID:705, profile.php:515).
B. Through the object: request data written to $order->ProfileStartDate,
or passed as an argument beside $wpdb, tainted the whole object and a
DIFFERENT field read from it came out tainted
(memberorder.php:1122/1633, pmproemail.php:262).
A path is now dropped when it passes through a call to a sink of its own
family, or when a field read is reached through the bare object after the
taint entered it by writing another field or without being assigned to
it. A member of a value that IS tainted (the result of a read, a return,
an assignment) and a field read back after being written are kept. Applied
to the sink query, the field join and the hook join. A real-Joern control
covers both cuts and both keeps, and fails on the old query; all 17 query
tests pass.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5.5
|
bdb5a866d6 |
revert(taint): the caller-fed field rule removed a true positive and missed the false ones
Reverts |
||
|
Marius CiepluchandClaude Opus 5.5
|
6ebbc89d64 |
fix(taint): a field is attacker input only when a caller feeds it request data
The field join (task 5.11) seeded every parameter of every method in the file, so every setter made its field a source. Adjudicated on PMPro, six of eight false SQL injections in a 1-in-5 sample were object fields holding database or internal values ($this->membership_id, $morder->user_id). A parameter now seeds the join only where some call site passes it request data without a sanitizer on the way (memoised per method and index); a request source read in the file still seeds as before. A real-Joern control covers both directions -- a setter only given a constant stays quiet, a constructor handed $_GET (the CVE-2023-6559 shape) is still found -- and fails on the old query with exactly the PMPro false positive. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
3c195c9369 |
fix(scan): itemise change context only in the files a change touched
The PHP transfer replay (pre-push 13.3) spent its whole 7,200 s case in the head side's evidence pass. A probe on one PMPro graph named the cause: not compute -- context adds ~20% engine time -- but OUTPUT. Each evidence request listed every method its scope reached, up to 1,845 rows; 200 requests into the bundled Braintree SDK returned 52,831 rows, 9.4 MB, against 211 rows without context, over 22,125 requests in a 3 GB container. `affected_context` only ever attaches a location in a changed file, and every other row was checked (real file under the root, usable extent) and discarded. The scan now sends the changed files; the engine itemises those and folds the rest into one `context_elsewhere` row carrying exactly what that check reads. With no safe file list the engine itemises everything, as before. Equivalence: unit controls over valid, missing-file, no-extent and outside-root cases, with and without a changed-file row; a real-Joern control; and on PMPro itself, 2,440 relationships and zero gaps decided identically from both forms, 52,831 rows down to 3,241 with the most referenced file as the change. Probe: benchmarks/push/context_cost_probe.py. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5.5
|
1b28465a61 |
feat(scan): size portions from measured per-answer cost and isolate the kill culprit
The 7.8 trace showed the completion loss was a sizer death spiral, not expensive families: after two kills the budget fell to 3 sink visits and never recovered, because portions finishing in 25-30 s sat under an assumed 70 s fixed start and read as "no information", so the budget could only shrink. - taint.sc streams each answer's own cost (`ms`); the backend carries it as a reserved `__timing__` list through shards, partitions and reuse. - The sizer budgets predicted seconds from per-family seconds-per-visit rates learned from those costs, and measures the fixed start as the wall time the answers do not explain (median), instead of assuming it. - A kill names its culprit: the first unanswered id in request order. The ids behind it never started and are re-queued at once; the culprit is asked alone after the rest. One that still fails alone after more than a portion's target is reported as `query_too_expensive`, apart from `query_failed`. - Portions grow id by id from the queue head, and a single-portion scan now goes through the queue too, so a kill there re-asks its remainder. - Fix: the reuse wrapper wrapped only `run_batch`, so since 7.8 every portioned query bypassed the graph-reuse cache and its source-root normalization. `run_batch_once` is wrapped too. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Fable 5.1
|
a8b226a5c8 |
feat(cpg): portion the work instead of capping it -- a kill keeps what finished
A hard per-invocation ceiling is all-or-nothing. A batch killed at 301 s lost every answer it had already computed, and the retry paid a fresh JVM start per part to ask them all again: measured, 116 requests killed at 300 s, then eight restarts carrying the same everywhere-sinks in every part. Three coordinated changes make portioning, not the cap, the thing that governs cost. The taint query STREAMS. The census first, then one line per request as it is answered, flushed as it goes, inside the unchanged fence. The other queries do not stream and need not; the driver reads both forms, and a legacy single document parses exactly as before. The driver HARVESTS a kill. The process group is terminated first -- a first cut read the pipe before killing and a pipe whose writer is alive never reaches end-of-file; the suite own hanging-engine control sat on that read for twenty-eight minutes -- then the pipe is drained and every complete line becomes an answer, marked partial. The retry asks only the remainder. Portion sizing ADAPTS. After each portion the observed seconds per sink visit, less the fixed start, resizes the rest toward a 120 s target; a portion that did not answer halves the budget rather than being fitted, because its true cost is unknown and above the ceiling. A portion cut down to fit puts its remainder back at the front of the queue. A census LINE carries nothing but the census. A first cut misread a legacy document that carried the census beside its answers as a census line and dropped every answer it sat beside; a control caught it. Task 7.8 in contributor-scan. Verified: 1,423 host tests pass, nineteen skipped, including new controls for the assembler in both forms and under a kill, for a retry that asks only the remainder, and for the sizer. Not yet verified: the joern-gated streaming check and the plugin measurement, which wait for the transfer replay to free the machine and are stated as open in the task. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5
|
2c5fba55ef |
fix(semantic-facts): reach the deserialization form WordPress actually writes
Across four WordPress plugins and 1,164 PHP files there is exactly one deserialization site, and
it is spelled maybe_unserialize. The bare token never reached it, and correctly so: the sink
matcher is word-bounded and the character before unserialize inside maybe_unserialize is a word
character. A token a repository never spells is a family that can never answer, which is what the
2026-09-19 census recorded for this one. maybe_unserialize is a real entry point rather than a
safe wrapper: it calls unserialize whenever the string looks serialized.
Also adds the prototype control. That family had never established anything on any subject, and
its only census report was the word `set` inside a comment. No repository available to this
project contains the mechanism: the only first-party matches on the Node subject are
app.set("view engine", "html") and its sibling, Express settings whose arguments are literals. So
the control demonstrates what the family can do, on a merge of the request body into an object,
and asserts the Express setting is not reported alongside it.
Corrects the PHP record under task 13.2. joernio/joern#6281, this project's own report, is a PHP
`global` inside a closure producing a node with two AST parents, which made the dataflow overlay
throw and cost the whole graph. It was filed against 4.0.623 and fixed by #6270 in v4.0.625,
which the image has pinned since before any measurement recorded here. Verified today on the real
trigger rather than on a transcription of the repro: PMPro's includes/fields.php, the file the
original bisection found, builds and applies the default overlays cleanly on 4.0.625 and on
4.0.631 alike. What remains under that task is a runtime failure and must not be re-diagnosed as
the graph bug.
1,417 tests pass, eighteen skipped.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5
|
e6bdac46e2 |
fix(access-control): decide the sibling scope, and see identity a destructuring spelled away
Two of NodeGoat's three corroborated claims were correct code. `const { userId } = req.session`
lowers to a temporary and then a field read, so nothing in the graph spells session.userId by the
time the value reaches dao.getByUserId(userId, ...), and the constraint the handler really applies
was invisible.
The arbiter now walks back three hops over the method's own assignments from the names the
operation is GIVEN, and req.session joins the identity sources. The token is only safe because of
that requirement: what discharges an object-level obligation is that the value passed to the
operation comes from the authenticated context, not that a session is mentioned nearby. A handler
that reads the session for logging and keys its query on a path parameter is the bug this family
exists for, and it stays unguarded.
Two exclusions the measurement forced, each caught by a page whose whole purpose is the leak it
demonstrates.
- A callback is not a constraint. Taking identifiers from the whole argument subtree credited
NodeGoat's memo listing with a discharge, because memosDAO.getAllMemos((err, docs) => ... userId
...) reads every memo in the collection and only RENDERS the session's user. Method references
are excluded from the argument names.
- An assignment is not a dominating guard. The session read dominates the query, so that clause
credited it instead and the memo listing went quiet again. An assignment is a read of the
authenticated context, not a check applied to it, so operator calls are excluded there -- the
same exclusion the sink matcher needed. The leak stays reported.
On NodeGoat displayContributions and displayProfile go silent, the allocations insecure direct
object reference, both benefits pages and the memo listing stay, and the login and signup handlers
become honestly unguarded rather than accidentally cleared by a callback's text. Six corroborated
where there were seven, and a better six. VAmPI is unchanged at two entailed findings.
The sibling scope is decided by measurement and kept at the file. On NodeGoat the region's own
file entails two handlers, both public routes and both false, and no true defect at all. Every
handler in the application entails six: four true, and the same two public routes. Both scopes
carry the same false positives, because neither can tell a public route from a protected one --
a semantic question that is already the corroborated band's residual. With the carried rung capped
the scope changes no verdict, only the witness, so widening it would buy a repository-wide walk
per question and no precision. A public-route discriminator would change that; the file scope is
not what holds this family back.
1,415 tests pass, sixteen skipped; thirteen Joern-gated query tests pass in the image.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5
|
c0427e6ceb |
feat(access-control): model the collection read, by argument shape rather than by name
NodeGoat's documented insecure direct object reference reads with allocationsCol.find(searchCriteria()), and `find` was absent from an operation table holding findOne, findById, findAll and findByPk. The obligation was missing at the commonest read there is, so that defect raised nothing at any point in the pipeline. It was absent for a good reason, and the reason is the argument, not the name. Measured on wp-statistics, a WordPress plugin that ships jQuery: its assets hold 88 calls named find, of which 70 pass a jQuery selector, 13 pass a predicate to Array.prototype.find and 5 pass a selector built at run time by a template literal or a concatenation. None is a collection read. So the obligation is claimed only where the first argument has the shape of a query document. The operation fact states which argument with query_document_arg, and the arbiter that can see argument shapes decides it: no arguments at all, an object literal, or a builder call are claimed; a method reference, a literal, an operator call and a bare identifier are not. Each exclusion is one of the measured classes above rather than a guess. Zero of those 88 calls became an obligated operation, and the zero is measured rather than empty: the same run's census reports 60 files and 4,552 methods, and the files holding those calls are in it. On NodeGoat all three find calls are collection reads and all three are claimed, which raises the obligation at four more handlers -- the allocations handler that takes userId from req.params on an authenticated route, the benefits pages whose isAdmin guard is commented out in the route table, and the memo listing. Seven of fifteen handlers now reach a verdict, all corroborated, none reported without a judge, and the census is unchanged on both subjects. A bare identifier is deliberately not claimed: col.find(query) and arr.find(pred) are the same shape and neither repository measured holds one, so this is a stated false negative rather than an unmeasured false-positive risk. 1,415 tests pass, sixteen skipped; twelve Joern-gated query tests pass in the image. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5
|
102a21fc4e |
feat(access-control): carry the obligation across the route-and-DAO split
The census measured this family completing 35 questions on NodeGoat with no obligation raised anywhere: every modelled data operation lives in app/data/*-dao.js, none in a route handler file, and operations were scoped to the region's own file. The family had nothing to be right or wrong about. The obligation now follows the one named call the handler writes, out of the region's file and no further. How it is followed is the point. jssrc2cpg does not resolve allocationsDAO.getByUserIdAndThreshold(...) to the DAO body: the callee is an external stub with no code. That stub's fullName records where the member was written, so the implementation is reached by the graph's own provenance rather than by a repository-wide name guess, which is the false positive the file scope was closed against. A control asserts it: a third module defining a member of the same name that nothing imports is not attributed. A row is attributed to the region's DECLARED function, which is what lets a question about displayAllocations match a graph that holds a <lambda>. A carried row is also checked for a guard at the CALL, because that is where a handler establishes who is asking. The method holding a carried operation joins the question's context scope, so the module holding the query cannot change under a claim that depends on it without the change being attributed. The witness names both sites and the finding is located at the call, which is the line a contributor changed. A carried obligation corroborates, it does not entail, and that was measured rather than assumed. Entailing it reported handleSignup, which looks a user up by name to check the name is free during registration and is public by design, because two authenticated siblings in the same file pass req.session.userId into the same module. The entailed rung's justification is that one FILE contradicts itself, same file and same trust context; carried, the compared operations live in a data-access module that handlers of different trust contexts share. The corroborated band already asks the right residual: does this operation require a check, or is the route legitimately public? Measured on NodeGoat: 15 handlers asked, obligations raised at zero before and six now, three reaching a verdict, all corroborated, none reported without a judge. VAmPI unchanged at two entailed findings, because it keeps handler and query in one file and carries nothing. What this does not reach is recorded, not implied. NodeGoat's documented insecure direct object reference still raises nothing, because its module reads with allocationsCol.find(...) and find is not a modelled operation: Array.prototype.find and jQuery's .find are everywhere, so it needs an argument-shape discriminator and its own measurement (task 14.8). Two of the three corroborated claims are correct code whose identity read a destructuring spelled away, and the cross-file contradiction that would entail the real defect needs a sibling-scope decision (task 14.7). 1,415 tests pass, fifteen skipped; eleven Joern-gated query tests pass in the image. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5
|
f31ef862a7 |
fix(output-encoding): ask about the server half, and say what the family cannot decide
The census recorded this family completing 44 questions on a Node application and finding
nothing, which is what a client-only vocabulary must do there: all six declared sinks were DOM
sinks. NodeGoat's cross-site scripting is server-side.
Decision: the family asks about a value reaching a rendering call, and states in its own
declaration that it cannot decide template-mediated escaping. Whether res.render is safe depends
on the engine's autoescape setting and on the template's markup, neither of which is at the
call, so res.render is not a sink and the family's limits field says so. A new CWE-79 sink
covers res.write, res.end, res.send and the response.* spellings; res.json stays out because an
application/json body is not a document.
Template auto-escaping is claimed where its evidence is, as a configuration defect:
swig.setDefaults, nunjucks.configure and marked.setOptions are security settings now.
It establishes a finding: app/routes/research.js:25, model_entailed,
`req.query.url + req.query.symbol -> res.write(body)`. The handler fetches a URL the caller
chooses and writes the response body into a text/html response, so the document served from
this origin is the attacker's. The record states how coarse the step is: the path from url to
body runs through the client's callback parameter, which the graph treats as reachable from the
call's arguments, so the conclusion rests on the call's semantics rather than on what the graph
established. Five of seven families now establish findings.
Four defects were found on the way, three of them false positives this change would otherwise
have shipped.
- The permissive-value set was shared across every setting, while a boolean's polarity belongs
to the setting: true leaves the door open in cors({origin: true}) and closes it in
swig.setDefaults({autoescape: true}). Adding the template engines would have reported the fix
as the defect on the line that offers it. ConfigSpec now carries per-setting values, and
Jinja2's Environment moved out of Python's shared list for the same latent reason.
- config.sc matched a dotted setting on its trailing segment alone, so marked.setOptions would
have claimed an editor's setOptions. dominance.sc already required the qualifier.
- taint.sc asked its flow query of every argument including the receiver at index 0. On a
response sink the receiver is the response object, so res.end() -- which takes no data -- was
reported exactly like the res.write(body) two lines above it. Flows now use the real
arguments, as arity already did.
- The arbiter sorted a file's settings by line number as TEXT, so line 126 preceded line 78 and
which setting a file reported depended on the spelling of its line numbers.
Template escaping disabled at server.js:135 is detected, measured model_entailed against the
real query and the real arbiter, and still not reported: the arbiter returns one verdict per
question and that file has an earlier permissive setting. Task 14.6 carries it rather than
hiding it.
1,415 tests pass, thirteen skipped; nine Joern-gated query tests pass in the image.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
|
||
|
Marius CiepluchandClaude Opus 5
|
57f5a3f0dc |
fix(semantic-facts): give untrusted destination the clients a Node server calls
The family census recorded this family completing 44 questions on NodeGoat, finding the open redirect, and having no way to ask about the repository's documented SSRF: needle.get on a query-string URL at app/routes/research.js:16. A thin vocabulary is a silent false negative. One data change, no query change. The declared set now covers axios, got, needle, request, superagent, unirest, undici and fetch as bare module tokens, plus http.get, http.request, https.get and https.request dotted. Bare is the documented call form of each module and covers its verb methods too, since a token matches word-bounded against the callee and axios is bounded inside axios.get; axios.get and axios.post are dropped as subsumed rather than unwanted. http and https stay dotted because their namespace also holds createServer, and a bare token would have claimed every server a repository starts. The request is now reported at app/routes/research.js:16, model_entailed, witness `req.query.url + req.query.symbol -> needle.get(url, ...)`. This family goes from one finding to two on NodeGoat, every other family is unchanged, and 299 of 299 questions still complete. Measured on that graph, needle matches exactly one call and the eleven other clients match none in first-party or vendored code, so NodeGoat shows the gap closing and cannot show those clients' noise. A control asserts both halves. A handler calling axios.get with a literal and needle.get with req.query.url reports the tainted client alone, and it reads the shipped vocabulary rather than a copy, so removing the client fails it. The CWE-601 redirect entry is untouched: a Location-header redirect is value-shaped and stays outside what this family asks. 1,411 tests pass, eleven skipped; seven Joern-gated query tests pass in the image. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Opus 5
|
4e2619e248 |
feat(pre-push-safety-net): census the declared families and fix a sink matched in a comment
Seven vulnerability families are declared and two had been proven. The census asks each of them, on whole checkouts of NodeGoat and VAmPI, whether it establishes anything at all, and reports a family that answered and found nothing separately from one that was never asked. Four establish findings: injection, access_control, config_secrets and untrusted_destination. Output encoding, path, prototype and deserialization establish nothing on either subject. All 299 NodeGoat and 35 VAmPI questions completed, so the silences are answers. Three of those silences are diagnosed gaps, not clean code. NodeGoat's documented SSRF calls needle.get, which the sink vocabulary does not hold. Its XSS is server-rendered, and output encoding declares only DOM sinks, so no question it can ask could have found it. Its two access-control defects raise no obligation at all: measured with the real dominance query, every modelled data operation lives in app/data/*-dao.js and none in a route handler file, while the family scopes operations to the region's own file. Tasks 14.2 to 14.5 carry each gap; none is fixed here. The census also found a false positive and it is fixed. NodeGoat's only prototype report was the word `set` inside `// set these up in case we have an error case`: the matched node is the <operator>.assignment holding the entire 1,000-character signup handler, and sinkMatches searched a node's whole text. It now confines the text clause to the callee and excludes operator nodes. Fourth instance of this bug class after fgets/gets, resolve/resolveUrl and user/users. Measured on NodeGoat's non-vendor calls, injection sinks fall 28 to 3, prototype 13 to 2 and untrusted destination 26 to 7, with no true positive lost; req.query had been an injection sink because `query` is word-bounded inside it. Witnesses now name the sink call rather than the assignment that contained it. 1,411 tests pass, ten skipped; the six Joern-gated query tests pass in the image, including a new one that fails on the shape this fixed. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo |
||
|
Marius CiepluchandClaude Fable 5.1
|
c955721f92 |
fix(cpg): the census overlay list is layer names, not characters
One flatten too many turned "base,controlflow,typerel,callgraph,dataflowOss" into "b,a,s,e,...", and every real build then warned that its graph lacked the dataflow layer while carrying it. The compile test now asserts the shape on a real graph. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG |
||
|
Marius CiepluchandClaude Fable 5.1
|
60963f565c |
fix(cpg): the real-request cost, attributed and removed -- overlays once, seeds scoped, data tables out
Task 5.13, measured on ten real pmpro pairs at callDepth 1 with each join half switched separately and
the 54 s JVM fixed cost subtracted: both halves off 11.1 s/request, field half 10.9 s, hook half past
49 s (timeout). Stage timers put it in `hookSeedsIn("")`: every field access in all 7,061 methods put
through the field join before the first callback was answered. Seeds are now the callback's own body
and file, which is where WP Statistics's `set_current_page` and `$this->rest_hits = ...` both live.
Both halves on: 9.7 s/request. `sourceNodes` is also materialised once per request instead of once
per sink.
The WP Statistics re-verification then exposed the larger term. A `joern --script` run is two JVMs and
`-J-Xmx` sizes only the launcher, so the heap knob governed nothing; the backend now sets
JAVA_TOOL_OPTIONS too. And since PHP builds went through php2cpg directly (
|
||
|
Marius CiepluchandClaude Opus 5
|
07df314e12 |
fix(cpg): prove the frontend can read the source before building, and check the graph always
Two permanent guards, and a correction that motivates both.
THE CORRECTION. There is no php2cpg defect. Every PHP build on this machine
ran through a local `php` shim proxying stdio through `docker run`, and
php2cpg depends on the INTERLEAVING of its parser's stderr banners with its
stdout JSON to attribute each document to a file. The proxy scrambles it:
streaming shim drops 72-88 767KB / 969KB / 1.2MB different every run
buffered shim drops 0 5,703 bytes empty, and silent
NATIVE php drops 0 2,872,996 bytes identical x3
The 637-file tree with native php: 0 drops, a 4,329,471-byte CPG, 20.0s,
1.9GB peak -- which is exactly what task 5.10 said before I started
"correcting" it. No intermittent failures, no malformed graph, no dataflow
assertion, no scale limit.
GUARD 1: the interpreter must prove it can read the tree, before anything is
built. Every instrument failure this project has had takes one shape -- the
input was unreadable and the tool reported a plausible ZERO -- and it has cost
four wrong diagnoses in a day:
a php that could not see ~/.cache -> "cannot handle 637 files"
a php whose stdio was proxied -> "drops half its batches"
a container missing a bind mount -> "0 dropped", 5,703-byte graph
None announce themselves: file_exists() is false, the parser writes nothing,
the frontend exits 0, and "no errors" reads exactly like success. Checking the
output cannot separate that from an empty repository. Checking that the input
arrived can, and costs one exec.
GUARD 2: the graph census is taken on EVERY build, not only one the frontend
warned about. A graph holding nothing arrives with no warnings at all --
silence is the symptom that has no symptom -- and the shortfall is now
reported as "the frontend reported no failures but the graph holds N of M
source files".
`cpg_build_failed` carries that detail through to the report instead of being
a bare reason.
Also adds tests/test_cpg_queries_compile.py: the queries are 900 lines of
Scala that nothing compiled. A rowsFor signature change shipped with 850 tests
passing and broke every query at runtime, found by a measurement failing hours
later. Each query is now compiled AND run against a real graph wherever joern
exists, and must return a parseable payload.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|