mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
602ec23e03d0d4f2a962ded814a9c74be7be9dc1
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Marius CiepluchandClaude Opus 5
|
922ed84053 |
feat(repos): a WordPress plugin with a verified CVE, and the WordPress DB API
contributor-scan 5.1's detection half. Paid Memberships Pro 2.9.7 pinned at
1a900be3, carrying CVE-2023-23488 -- unauthenticated SQL injection through
the `code` parameter of the /pmpro/v1/order REST route.
**Verified in the code, not taken from the advisory.** At this commit
classes/class.memberorder.php:936 reads
$id = $wpdb->get_var("SELECT id FROM $wpdb->pmpro_membership_orders
WHERE code = '" . $code . "' LIMIT 1");
and tag 2.9.8 replaces it with `$wpdb->prepare( "... code = %s ...", $code )`.
637 PHP files and 117k lines, so it is also the first PHP checkout large
enough to say anything about an envelope.
**WordPress never calls mysqli; it goes through $wpdb**, so the facts could
not have found this. Added qualified: `$wpdb->get_var`, `->get_results`,
`->get_row`, `->get_col`, `->query`, `->replace`. Qualified because a bare
`get_var` or `query` matches any method of that name anywhere -- the
generic-name trap that made `query.get` match every `.get(` in a repository
this morning. `->insert`, `->update` and `->delete` are deliberately absent:
they take arrays and WordPress prepares them itself.
**And the thing that nearly went wrong.** The obvious next step when
writing PHP facts is to add WordPress's sanitizers -- sanitize_text_field,
esc_html, esc_attr. Doing so would have made this CVE invisible, because
its handler calls `sanitize_text_field( $params['code'] )` and then
concatenates the result into a query. sanitize_text_field is a real
sanitizer that does nothing whatsoever about SQL injection.
`TaintSpec.sanitizers` is one flat list per language whose comment states
the assumption outright -- "a sanitizer that breaks one flow breaks it
whatever the sink's family" -- and PHP falsifies it. So only SQL-specific
cleansers are listed, which leaves XSS under-sanitized instead, and the
real fix is recorded as task 5.6: a discharge counts for the obligation it
discharges. That is task 2.6's finding arriving in the taint family, where
the dominance arbiter already reads kind from the facts and the taint spec
has no equivalent.
The end-to-end scan of this checkout is not yet reported: the first run
returned 1577 regions with 0 scanned, and the CPG builds standalone at
4.3MB with no errors, so the two do not yet agree. Numbers when they do.
Gates byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
61811512a8 |
feat(facts): PHP semantic facts, validated against a real PHP CPG
contributor-scan 5.1, the first step toward the v0.1 WordPress target. Written against a PHP CPG rather than against PHP source, because the two do not look alike. Verified with joern-parse 4.0.623 / php2cpg on a probe: $_GET["name"] -> call `<operator>.indexAccess`, CODE $_GET["name"] mysqli_query($conn, $sql) -> call NAMED mysqli_query echo $_GET["msg"] -> call NAMED echo So sources match on code and sinks match on name, and every entry the probe did not exercise is marked unverified in the file -- `include` and `require` among them, which is PHP's most characteristic path bug and the first thing to check on a real checkout. Five families derive (injection, output_encoding, path, deserialization, untrusted_destination) with no sink that is its own sanitizer -- asserted by a test, because that bug class has now appeared three times in three matchers. `php` is added to the region model's languages, without which the facts would have reached nothing. The arbiter decides 4 of 4 on the probe: the SQL concat entails, its `mysqli_real_escape_string` twin drops to corroborated, `echo` XSS entails and `system` command injection entails. **A whole scan finds 2 of those 3, and the gap is the finding.** PHP files yield FILE-LEVEL regions because no PHP entry points are mapped, and the taint arbiter returns one verdict per region per family -- so a file-level region reports at most one injection finding however many it holds. `system` won on flow length and the SQL injection went unreported. That is the mirror image of task 2.9: that one is many regions collapsing to one site, this is many sites collapsing to one verdict, and it bites hardest exactly where PHP needs it not to, since a WordPress file is large and has no decorator for the mapper to find. Recorded as task 5.4 with WordPress's registration model (add_action, add_filter, admin-ajax, REST routes), which the mapper already handles in principle for other frameworks. A 13-line probe is not a WordPress result. What is established is that the seam works end to end for PHP; everything about scale and coverage is unmeasured, and there are still zero PHP pairs in a corpus of 263 python, 351 c_cpp, 47 javascript and 49 java. Gates byte-identical. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG |
||
|
Marius CiepluchandClaude Opus 5
|
67497c4d68 |
fix(cpg): a sink is matched as a word, not as a substring
`fgets` contains `gets`. The sink matcher's last clause was an unbounded
`methodFullName.contains(n)`, so every bounded read in libpng matched the
sink for the unbounded one. All 26 entailed findings on that repository
were the same false positive:
*argv -> fgets(pnmline, 256, wpng_info.infile) (line 335, 8 steps)
which is the safe API being reported as the dangerous one -- and `fgets`
is in the fact tables as a SOURCE, never a sink. Verified against the CPG:
methodFullName is exactly "fgets", and .contains("gets") is true.
Third instance of one bug class in this codebase, after the sanitizer that
matched `resolve` inside `resolveUrl` and the discharger that matched
`user` inside `users`. The sink matcher had never been given the same
treatment. All four of its clauses now run through the shared bounded
matcher, and that matcher is the punctuation-tolerant version written for
task 2.6 -- so `input(`, which an unconditional lookahead had made
unmatchable in `input(x)`, works for the first time as well.
libpng entailed 26 -> 0
Zero is the right answer for a PNG library whose only known CVE is out of
family. Gates byte-identical, so no pair outcome depended on the substring
match.
What remains on that scan is recorded, not claimed: 113 `c-unsafe-*` rows
assert an API is present rather than misused, and 50 overlay-coverage rows
are annotations rather than findings. 82 of the 113 are under `contrib/`.
Task 2.11 covers whether that band is worth putting in front of a
contributor at all; 2.10 already covers not scanning example code.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
d15985f695 |
fix(model): a guard discharges the obligation it is a guard of
contributor-scan 2.6. Access control entailed 7 of 9 VAmPI handlers,
including three that do authorize and three the OpenAPI spec declares
public. Five separate defects, each invisible to a corpus of one-function
excerpts and each found by asking the query what it actually matched.
**A trailing segment is not a match.** `query.get` was matched by its last
segment, so every `.get(` in the repository became a protected database
operation -- `request.headers.get('Authorization')` and
`request_data.get('username')` included. Four of the seven false positives
came from this, and so did the apparent detection of the known BOLA: it was
entailed because fetching the Authorization header had no dominating guard,
not because of the defect. A qualified token now requires its qualifier.
**Configuration is not authorization.** dominance_specs took every
obligation fact. `security_setting` operations are discharged by a VALUE,
which config_specs already claims them for; pooled in here they made
`app.run(...)` an operation requiring an authorization guard and put
thirteen tokens in BOTH lists -- an obligation that is its own discharge,
the same shape as the taint sink that used to sanitize itself.
**Input validation is not authorization.** `jsonschema.validate` was
counted as a guard, so handlers with no authorization check at all read as
guarded. No operation in the table requires validated_input.
**A guard discharges the obligation it is a guard of.** The largest one.
`orm-read` requires an identity_constraint or an ownership_check;
`token_validator` is a path_guard. Flattened into one list, authenticating
the caller discharged an object-level obligation -- a handler that
authenticates and then looks the record up by a PATH parameter read as
fully guarded. The facts state that relation and the spec was discarding
it; it is now carried and the engine asks per operation.
**A word boundary needs a word.** Found while fixing the above. Guard
matching wrapped every token in `(?<![A-Za-z0-9_])...(?![A-Za-z0-9_])`.
The identity token `['sub']` begins with `[`, and the character before it
in `resp['sub']` is `p` -- so the assertion could never hold. Every
identity token starting with punctuation was unmatchable, and the identity
constraint that separates a correct lookup from an IDOR was invisible.
Also: dominance rows were repository-wide, so the model's unguarded
`delete_user` was attributed to the API handler of the same name. Rows are
scoped to the region's file, which is what the rung's own justification
says -- the evidence is a file contradicting itself.
Measured against VAmPI's OpenAPI `security` blocks, which declare per
operation whether auth is required, and agree with the code:
vampi precision 28.6% (2/7) -> 50.0% (2/4)
known found 3 of 3, all model_entailed
And on the labelled access-control pairs, which nobody tuned this against:
all 9 pairs pair_correct 3 -> 3, leak 44.4% -> 11.1%
7 non-VAmPI pair_correct 2 -> 2, leak 42.9% -> 0.0%
Every detection lost was on a pair whose FIXED side was flagged too, which
scores nothing. No correct detection was traded for the precision.
get_by_title was flagged before it was in the recipe: its code carries the
comment "# Broken Object Level Authorization" and the README lists it, so a
real finding was being scored as a false positive. It is now the recipe's
third known vulnerability.
Both remaining false positives are endpoints the spec declares public.
Recorded as 2.8, with the reason the mapper's access_level cannot simply be
trusted: for decorator frameworks "public" means "no decorator found",
which is the bug itself.
Gates byte-identical.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
fe45081b43 |
fix(ci): run ruff format, and record run 5 at 90% pair-correct
CI was red on `ruff format --check`: 23 files. I ran `ruff check` after every
change this session and never once ran `ruff format --check`, though CI runs
both. Lint passing is not formatting passing, and I treated one as evidence for
the other for the whole session.
Formatting applied. The full CI sequence now reproduces locally: format, lint,
mypy over 94 files, 735 tests, three gates byte-identical, compileall, and the
semantic job's 16 marked tests.
Also records run 5 of the pipeline measurement, which the vampi fix lifted as
predicted:
hunter (K=5) pipeline (K=1)
pair-correct 76.7% 90.0%
leak 12.3% 6.7%
detection -- 96.7%
cost ~$0.69 $0.0068
27 of 30 pairs. Five runs, each step from a measurement contradicting an
assumption: 73.3 (no pipeline path) -> 73.3 (routing fixed, IDENTICAL, which
exposed the discarded verdict) -> 80.0 (residual question matched to its
arbiter) -> 86.7 (aiohttp settings) -> 90.0 (dominance question no longer
contradicting its own evidence).
One miss left: dsvpwa-dsvpwa-main, whose setting is an argparse --ssl default
rather than a security-setting call. Out of reach before the settings work and
still is; contorting the arbiter around one pair would be worse than the miss.
The caveat is recorded beside the number: 30 pairs of one slice, and the slice
iterated against all session. The +16% overfitting gap against vfc-js is why
contributor-scan Req 8.1/8.2 exist.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
eebca3f663 |
fix(model-grounded-detection): the absence arbiter was silencing its own family
Two bugs, together making 4.1 report "no bug" on a textbook IDOR. Both found by running it against a real corpus pair rather than a fixture. 1. constraint_params were treated as dischargers. A field named `user_id` is what an IDOR is MADE OF -- `filter_by(id=user_id)` with user_id off the request is the bug, not the fix. The deleted checker only counted an identity constraint whose value's PROVENANCE was the authenticated context, so the parameter name alone evidences nothing. This is the mirror of group 2's sanitizer bug: a thing that must never count as a discharge, counted. 2. Guard tokens matched as substrings. With `user` in the token list, the string "SELECT * FROM users WHERE id = ?" contains `user`, so the query reported every operation in the handler as guarded. Matching is word-boundary now. On threatbyte-api-v1-delete the arbiter previously found three operations, all "guarded" by evidence like '"SELECT * FROM users WHERE id = ?"' and '(user_id,)', and returned no verdict at all. After the fix, on the real pairs: threatbyte-api-v1-delete vuln=model_corroborated fixed=none vampi-users-update-password vuln=model_corroborated fixed=none Both distinguished, deterministically, with no model call. Corroborated rather than entailed is right for these: they are single-handler excerpts with no guarded sibling to be inconsistent with, which is the asymmetry the rung distinction exists to express. identity_sources still discharge; request_sources and permissive_values remain excluded because they describe the vulnerable shape rather than the fix. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG |
||
|
Marius CiepluchandClaude Opus 5
|
fec67253f3 |
feat(model-grounded-detection): 4.1 guard dominance, 4.2 the missing sink facts
4.1 is the arbiter for the class no crash oracle reaches. An access-control bug
is the ABSENCE of a guard: no flow to follow, no crash to reproduce. What the
model can establish is a consistency violation -- an obligated operation no
discharging guard governs, in a file where its siblings are governed by one.
"Governed" turned out not to be one relation, and asking pure CFG dominance of
the operation gets two of three shapes wrong. Measured against the real engine:
identity constraint the guard is INSIDE the operation's arguments
filter_by(owner_id = current_user.id)
identity branch abort(403).controlledBy == [note.owner_id != current_user.id]
-- the check runs AFTER the fetch, so it never dominates
the operation, but it controls whether the value escapes
dominating guard a discharger-naming call that dominates the operation
That is why the query uses controlledBy rather than dominatedBy for shape 2.
Two bugs found by running it, both the same class as the sanitizer bug in group
2 -- part of a fact flattened, the part carrying the signal dropped:
* dominance_specs used only `calls` and `decorators`, omitting
identity_sources (current_user, request.user) and constraint_params
(owner_id, user_id) -- which is how an identity constraint is actually
written. The arbiter reported "no guard" on handlers that plainly had one.
request_sources and permissive_values stay excluded: they describe the
vulnerable shape, and including them would let the arbiter read a bug as
its own fix.
* matching operations by code containment yielded four or five desugared rows
per operation (the bare `tmp1.filter_by` receiver among them, carrying none
of the guards the real call has), so a guarded operation could be flagged.
Matching is by call name now.
Live, deterministic, zero model calls:
leaky -> model_entailed no discharging guard, while 2 sibling handlers are guarded
edit_note -> no finding (ownership check controls the denial)
list_notes -> no finding (identity constraint in the query)
4.2 authors the sink facts the go/no-go measured as its largest miss class (11
of 23): Django ORM raw SQL, and the four families that previously had no fact
routing to them by CWE at all -- path, output_encoding, untrusted_destination,
prototype. Each entry is a documented dangerous API of the named library with
its provenance recorded inline, and rule_ids left empty because a sink fact is
not a detection rule. Flagged for maintainer review as data, given the standing
"no LLM-authored patterns" constraint. The exact-set assertion written in task
1.1 to hold this gap open is updated rather than deleted, so it still fails if a
family ever loses its seed.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
f59e0f5b5a |
fix(model-grounded-detection): two modelling bugs the first live Joern run exposed
Both were found by running the real engine, and neither was visible offline.
1. A sink was its own sanitizer. The shipped facts mark some sanitizers with a
*shape* qualifier -- parameterized ("execute(sql, params) binds rather than
interpolates") and literal_format_arg ("printf with a constant format string
is safe"). These describe a safe FORM OF THE SINK CALL, not a call that
cleans a value on the path. Flattening them into the sanitizer list made
`execute` both the injection sink and its own sanitizer, so every SQL flow
would have reported as already-clean and nothing could ever be entailed.
Fixed as a class, not an instance: any fact carrying a shape qualifier is
excluded from `sanitizers` and kept in `safe_shape_sinks` for the shape test
task 4.2 will model properly. Fixing only the Python `parameterized` case
would have left C's entire printf family -- which the generalised test then
caught immediately.
2. labeled_family lost its deterministic derivation in group 1. The deleted
classifier resolved a pair's family by declared label, else obligation, else
mechanism, else CWE, and only asked an LLM when every link failed -- which
for a labeled corpus was never. Group 1 reduced this to the declared label
alone, which silently emptied every family whose pairs are labeled by CWE:
the injection slice went to zero pairs. The chain is restored, still with no
model call, and the family census again matches the committed entailment
ceiling exactly (injection 50, memory 176, access_control 8, ...).
Also: the Joern seam now runs in the CPG's own scratch directory, because
`joern --script` writes a `workspace/` beside the working directory and would
otherwise leave one inside the repository being analysed.
The taint query itself was corrected against the live engine: the sink is the
call's ARGUMENTS (data flows into arguments, not into the call node) and
reachableByFlows is called on the sink with the source. With that, the
three-hop chain our flat IR could not resolve is found:
request.args["cmd"] -> cmd -> "echo " + cmd -> full -> os.system(full)
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG
|
||
|
Marius CiepluchandClaude Opus 5
|
9ed0e45c5f |
feat(model-grounded-detection): group 1, subtract the noise architecture
99 source modules -> 81. 832 tests -> 646. The whole learning/ package (16 modules, ~4,100 lines) and six semantic/ modules removed, with the three gates byte-identical to 2026-09-06-gate-baseline.txt at every step. The sequence is what made it safe: vocabularies ported first (1.1), importers detached second (1.2-1.3), deletion third (1.4). Every import break during the deletion was therefore a detachment miss in the importer and was fixed there -- no module was restored to satisfy one. Both misses were the same shape and were fixed as a class: operations.py and shapes.py each imported trailing_name from the deleted variants.py, so it moved to semantic/ir.py, its natural home, and both importers changed in one edit. Ported before deleting: the closed guard vocabulary (verbatim, nothing else holds it) into model/specs.py, which derives TaintSpec and DominanceSpec from the retained ruleset facts rather than copying them. Re-homed: families -> model/taxonomy, candidates -> model/candidates, endpoint -> model/endpoint, plus Region, labeled_family and labeled_spans. labeled_family no longer falls back to the LLM classifier -- an unlabeled pair is honestly unknown. Four deviations, each recorded in the spec notes with its reason. improve/ evolve.py and validator.py were stripped, not deleted: each mixes the mechanism lever (gone) with the deterministic rule-status loop `ousast improve` still runs, which has no LLM in it; a test now asserts the lever's absence by symbol so it cannot creep back. The learning CLI group became `model candidates`, because deleting it outright would have orphaned the retained enumerator by removing its only entry point. thinking moved from [learning] to [models]. A gap named rather than papered over: no retained sink fact routes by CWE to path, output_encoding, untrusted_destination or prototype -- the same hole the entailment ceiling diagnosed. The test asserts the seeded set and the gap set exactly, so task 4.2 must update it when it authors those sinks. The audit found exactly one genuine orphan: vectorstore.py, a bakeoff harness with no production caller and no CLI, alive only through its own test. The five named subsystems each carry an explicit decision. Manifest committed, with a test asserting the tree still matches it. Group 2 (the Joern go/no-go) has not started. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01T7XD7x5KonMdmGRkp63EQG |