diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index bfdf73d..f7c96c1 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -502,7 +502,7 @@ - _Requirements: 4.4, 8.1, 9.1_ - _Depends: 5.1_ -- [ ] 5.8 WordPress's popular flaw classes, on popular plugins +- [x] 5.8 WordPress's popular flaw classes, on popular plugins - > "in wp focus on some of their popular flaws and plugins. these are the keys to unlock practical > software verification." - One plugin is a checkout; a class is a claim. WordPress's recurring shapes are narrow and well @@ -518,6 +518,26 @@ advisory the way `pmpro.toml` does -- the CVE read out of the code, not out of the advisory text. - Observable: a class-by-class table over several plugins, saying for each whether it was found, missed, or out of scope. That is the artifact worth teaching from; a single found CVE is not. + - **Done 2026-09-09.** The table is in `benchmarks/repos/README.md`, "WordPress, class by class". Three + plugins pinned, each CVE read out of the code, each measured against its own fixed commit: + + | class | plugin, CVE | verdict | + |---|---|---| + | unauthenticated injection, sink in the receiving function | `pmpro`, CVE-2023-23488 | **found**, pair separates | + | arbitrary file operation | `mwwpform`, CVE-2023-6559 | **found**, pair separates | + | unauthenticated injection across a WordPress hook | `wpstatistics`, CVE-2022-25148 | **missed**, structurally | + | missing capability check | — | cannot be asked (5.9) | + | unescaped output | — | cannot be trusted (5.6) | + + Both hits are entailed by the graph alone and both disappear on the fixed side, because both fixes are + calls the fact tables carry. Precision is not clean and the table says so: PMPro's vulnerable side + reports 15 entailed, 4 from a named parameter and 11 from the receiver, with one demonstrable false + positive surviving into 2.9.8. + - The miss is the useful half. WP Statistics' source and sink are both modelled and the fix is `esc_sql`, + yet nothing is reported on either side, because the only edge between the two files is + `add_filter('...', array($this, 'method'))` / `apply_filters('...')` -- both ends naming the callback with + a **string**. No frontend can draw that edge. Most WordPress plugin data travels this way, which makes it + the largest structural gap for PHP and not a fact-table problem. Recorded as 5.11. - _Requirements: 4.1, 4.4, 9.1_ - _Depends: 5.7_ @@ -547,11 +567,56 @@ exact verified line. 16 entailed injection findings, and the file holds 26 `$wpdb` query calls of which **zero** use `->prepare`, so the engine under-reports rather than over-reports. See `benchmarks/measurements/2026-09-09-pmpro-cve-found.json`. - - Still open: the 637-file boundary itself, and whether the REST handler two calls away can reach it -- - this slice found the sink through a same-function flow, not across the object. + - **Root cause found 2026-09-09, and it was never a scale limit.** `php2cpg` 4.0.623 reads its PHP parser's + stdout and stderr as one merged stream. The parser writes a `====> File :` banner to stderr the + moment it starts the next file, while the previous file's multi-megabyte JSON is still draining from a + block-buffered stdout; the banner lands inside the JSON, ujson reports `expected json value got "="`, and + that file is dropped. Reproducible in three commands: + + class.memberorder.php alone (1834 lines) -> 142KB CPG, no failures + the same file + a two-line file -> 5.7KB CPG, BOTH dropped, exit status 0 + + One oversized file empties the whole graph, and `joern-parse` propagates none of the warnings -- thirteen + lines of output, exit 0, "Successfully wrote graph". It is also a **race**, not a threshold: five builds + of the same two-file input gave three graphs and two empty ones. + - Two things follow. The engine now says so (`cpg_empty`, from a census carried on the batched query; see + `cpg/backend.py`), which is what turned this from a clean bill of health into a diagnosis. And the + mitigation is per-file, not per-repository: `--exclude` the file whose dump is oversized and the rest of + the tree builds, so the coverage disclosure should name the excluded files rather than the build failing. + - Still open: wiring that exclusion loop, the 637-file build with it in place, and whether the REST handler + two calls away can reach the sink -- the slice found it through a same-function flow, not across the + object. - _Requirements: 4.2, 4.3, 9.1_ - _Depends: 5.7_ +- [ ] 5.11 The WordPress hook edge, or an honest statement that it is not drawn + - `add_filter('name', array($this, 'method'))` and `apply_filters('name', $value)` are a data-flow edge that + no CPG frontend draws, because both ends name the callback with a string. 5.8 measured the cost: WP + Statistics' CVE-2022-25148 has a modelled source, a modelled sink and a modelled fix, and is invisible on + both sides purely because the path runs through this edge. + - The registry is static and small. Every `add_action`/`add_filter` in a plugin names a hook and a callback, + and the mapper already parses both for entry points -- so the pairs can be built without any inference, + and the question is only where to apply them: a synthesized call edge in a preprocessing pass, or a + source/sink pairing the taint query is told about. + - Do NOT approximate it with "any hook may reach any handler". That is a complete graph over the plugin and + would entail everything. + - Observable: CVE-2022-25148 entailed on `wpstatistics`'s vulnerable side and absent on its fixed side -- + the pair already pinned, already measured as a double miss, so the number moves or it does not. + - _Requirements: 4.4, 8.1, 9.1_ + - _Depends: 5.8_ + +- [ ] 5.12 Taint a field where it is assigned + - Excluding `$this` from parameter sources cuts PMPro's order class from 16 entailed findings to 6, all + named-parameter sourced, and makes its pair separate cleanly -- and loses CVE-2023-6559 outright, because + MW WP Form's `_delete_files()` takes no parameters and its attacker-controlled path arrives as + `$this->attachments`. The receiver was kept and the measurement written next to it in `taint.sc`. + - The right fix is field-sensitivity: `$this->attachments` should be tainted at the assignment that fills + it, so a method with no parameters is not either universally tainted or universally clean. + - Observable: CVE-2023-6559 still entailed with the receiver excluded, and PMPro's receiver-sourced count + down from 11. + - _Requirements: 6.1, 9.1_ + - _Depends: 5.8_ + ## Group 6 — Regression baselines - [ ] 6.1 Repository baseline and delta diff --git a/benchmarks/repos/README.md b/benchmarks/repos/README.md index 17f0769..3270f3c 100644 --- a/benchmarks/repos/README.md +++ b/benchmarks/repos/README.md @@ -55,6 +55,101 @@ HEAD today and not at the pinned commit, where it is still `LICENSE`. |---|---|---|---|---| | `vampi` | python | 520 lines, 11 files | detection | Small enough that a detection result is not confounded by budget or ranking. Its routes are registered through an OpenAPI `operationId` rather than decorators, so it exercises registration-based entry-point mapping. Excerpts of it are already in the pair corpus, so a whole-repository run says directly whether a verdict survives the move off the excerpt. | | `libpng` | c | 91k lines, 101 files | envelope | Whether one CPG over a real C library is affordable at all: build time, query time, peak memory, and whether the ranker yields a sane number of regions. Its CVE is out of scope by construction. | +| `pmpro` | php | 117k lines, 637 files | detection, envelope | Paid Memberships Pro 2.9.7. CVE-2023-23488 is an unauthenticated SQL injection whose value reaches the sink inside the function that receives it -- the shape the PHP fact tables were built for. 2.9.8 wraps it in `$wpdb->prepare`, so the pair separates. | +| `mwwpform` | php | MW WP Form 5.0.3 | detection | CVE-2023-6559, an arbitrary file deletion. The fix adds `realpath()`, `is_file()` and a directory-confinement test, all of which the facts already carry, so this is a pair the engine can separate rather than merely flag. | +| `wpstatistics` | php | WP Statistics 13.1.x | detection | CVE-2022-25148, an unauthenticated SQL injection that crosses `add_filter`/`apply_filters`. Pinned because it is NOT expected to be found: a table of hits teaches nothing about where the engine stops. | + +## WordPress, class by class + +WordPress's recurring flaw classes are narrow and well documented, which makes them a fair test: each one +maps to a family this engine already has, so "not found" has to name a mechanism rather than shrug. Three +plugins, each pinned at a version whose advisory was **read out of the code** and not out of the advisory +text, and each measured against its own fixed commit. + +| class | plugin, CVE | vulnerable side | fixed side | verdict | +|---|---|---|---|---| +| Unauthenticated injection, sink in the function that receives the value | `pmpro`, CVE-2023-23488 | entailed at `class.memberorder.php:936:getMemberOrderByCode`, sourced from `$code` | absent | **found**, and the pair separates | +| Arbitrary file operation | `mwwpform`, CVE-2023-6559 | entailed at `class.mail.php:259:_delete_files` | absent | **found**, and the pair separates | +| Unauthenticated injection, value crosses a WordPress hook | `wpstatistics`, CVE-2022-25148 | not reported | not reported | **missed**, structurally | +| Missing capability check | — | — | — | **cannot be asked**: PHP has no obligation facts | +| Unescaped output | — | — | — | **cannot be trusted**: the sanitizer list is flat | + +### The two it finds + +Both are entailed by the graph alone — no judge, no model call, no spend — and both disappear on the fixed +side because the fix is a call the fact tables already carry: `$wpdb->prepare` for one, `realpath()` for the +other. That is the distinction worth teaching. The engine is not recognising a bug; it is failing to find a +cleansing call on a path it resolved, and then finding one. + +The precision is not clean, and saying so is the point. On PMPro's order class the vulnerable side reports +**15 entailed findings**, of which **4** are sourced from a named parameter and **11** from the receiver, +`$this`. One receiver-sourced finding survives into 2.9.8 at a query whose every fragment is wrapped in +`esc_sql` — a false positive, and a legible one: the flow reaches the sink through `$this->sqlQuery`, so the +sanitizer is on the fragments and not on the path. + +Excluding the receiver was tried and reverted. It cuts that file to 6 findings, all named-parameter sourced, +and makes the PMPro pair separate cleanly — and it **loses CVE-2023-6559**, because `_delete_files()` takes +no parameters at all and its attacker-controlled path arrives as `$this->attachments`, assigned by another +method. Trading a verified CVE for findings merely suspected of being false is the wrong trade. The real +defect is field-sensitivity: a field should be tainted where it is assigned. + +### The one it misses, and why + +WP Statistics' CVE is an unauthenticated REST hit whose `current_page_id` is interpolated into +`$wpdb->get_row`. Every piece is in scope — a modelled source, a modelled sink, and `esc_sql` as the fix — +and the engine still reports nothing, on both sides. The reason is checkable rather than inferred: + +* the file holding the sink contains **zero** superglobals; +* it makes **no** call into the class holding the source; +* the only connection between them is + + ```php + add_filter('wp_statistics_current_page', array($this, 'set_current_page')); // hits.php:43 + return apply_filters('wp_statistics_current_page', $current_page); // pages.php:105 + ``` + +Both ends name the callback with a **string**. No frontend can draw that edge, so no dataflow engine can +cross it, and the honest description is not "we missed it" but "the graph does not contain the path". Most +plugin data travels this way. It is the single largest structural gap for PHP, and it is not a fact-table +problem. + +What the engine reports on that plugin instead is two other sites — `getTop:406` and `TotalCount:443` — the +same weakness class at the wrong lines, identical on both sides, so the pair does not separate. They are not +noise: upstream now annotates both with `// phpcs:ignore WordPress.DB.PreparedSQL...`, and later rewrote +one of them with `$wpdb->prepare`. Right class, wrong sites, no discrimination. + +### The two it cannot ask + +`dominance_specs(language="php")` returns an **empty** mapping. There are no PHP obligation facts, so the +missing-capability class — `current_user_can`, `check_admin_referer` — is never asked, on any plugin, and +would report nothing however many were added. That is not a miss; nothing was measured. Recorded as 5.9. + +`output_encoding` for PHP has sinks (`echo`, `print`, `printf`) and shares the one **flat** sanitizer list +with every other family. That list contains `esc_sql`, `intval`, `realpath` and `escapeshellarg`, none of +which escape HTML, and excludes `esc_html` and `esc_attr`, which are the only ones that do. So it is wrong in +both directions at once: it would accept `echo esc_sql($x)` as safe and report +`echo esc_attr(sanitize_text_field($_REQUEST['login']))` — the pattern WordPress documents as correct, and +which appears verbatim at `pmpro/includes/login.php:684` — as a finding. Until a sanitizer can say which +weakness class it cleanses, no XSS verdict from this engine means anything. Recorded as 5.6. + +### What nearly invalidated all of it + +The first PMPro measurement in this series reported 64 regions examined, zero findings and zero degradations. +The graph it examined had no methods in it. + +`php2cpg` 4.0.623 reads its PHP parser's stdout and stderr as one merged stream, and the parser writes a +progress banner to stderr the moment it starts the next file — while the previous file's multi-megabyte JSON +is still draining from a block-buffered stdout. The banner lands inside the JSON, the file is dropped, and +`joern-parse` propagates none of it: thirteen lines of output, exit status 0, "Successfully wrote graph". +One oversized file empties the entire graph, so this was never the scale limit it looked like. + +It is also a **race**, not a threshold. Five builds of the same two-file input: three produced a graph, two +came back empty. Before the batched query started carrying a census of the graph, two of every five scans of +that plugin would have reported a clean bill of health. + +That is the argument for why a corpus of benchmarks is not enough on its own, made concretely. Every gate +stayed byte-identical through all of it. A corpus tells you a verdict is right; it cannot tell you the +verdict was reached by looking at anything. ## Baselines