diff --git a/.kiro/specs/pre-push-safety-net/status.md b/.kiro/specs/pre-push-safety-net/status.md index cc68981..7530bbb 100644 --- a/.kiro/specs/pre-push-safety-net/status.md +++ b/.kiro/specs/pre-push-safety-net/status.md @@ -196,6 +196,16 @@ pinned that version throughout; it is verified today on the real trigger file, o failure take longer. The engine pin stays where it is, because the newer release measures the same here. +That runtime reading was itself incomplete, and the correction matters more than the reading. +PHP is not silent: at 25 and 100 regions PMPro completes with no timeout at all and establishes +19 and 43 findings, including a true-positive SQL injection at +`adminpages/discountcodes.php:749` of exactly the class the PHP fact table predicted. It looked +silent because the census harness ordered its verdict on question completion before findings, +and because only the 500-region runs had been examined. The harness is fixed. What remains is a +scale limit of roughly one to two seconds per question, and one real gap that is bookkeeping +rather than detection: no PHP question is ever marked completed, even in the runs that find +things. + Closing the second gap surfaced four defects, three of which were false positives it would otherwise have shipped: a permissive-value set shared across settings whose booleans have opposite polarity, a dotted setting matched on its trailing segment alone, a flow query asked of @@ -211,4 +221,5 @@ Evidence: [family detection census](../../../benchmarks/measurements/2026-09-19- [collection read obligation](../../../benchmarks/measurements/2026-09-20-collection-read-obligation.json), [sibling scope and identity](../../../benchmarks/measurements/2026-09-20-sibling-scope-and-identity.json), [every permissive setting](../../../benchmarks/measurements/2026-09-20-every-permissive-setting.json), -[PHP runtime after the graph fix](../../../benchmarks/measurements/2026-09-20-php-runtime-after-graph-fix.json). +[PHP runtime after the graph fix](../../../benchmarks/measurements/2026-09-20-php-runtime-after-graph-fix.json), +[PHP detection corrected](../../../benchmarks/measurements/2026-09-20-php-detection-corrected.json). diff --git a/.kiro/specs/pre-push-safety-net/tasks.md b/.kiro/specs/pre-push-safety-net/tasks.md index 3fdea37..e65e0aa 100644 --- a/.kiro/specs/pre-push-safety-net/tasks.md +++ b/.kiro/specs/pre-push-safety-net/tasks.md @@ -459,6 +459,12 @@ Preserve all completed task evidence below. - _Retried 2026-09-18 with batch splitting, still not established._ A batch that fails whole is now retried in eight parts, which a probe made look promising: the evidence pass answers all 4,425 PMPro regions in one batch in 29.7 s with a 681 KB response, and cost is fixed rather than per request, so neither count nor size is near a limit. That probe sent `evidenceOnly=true` and therefore measured the structural pass, not the failing one; recorded because those numbers would otherwise read as evidence that PMPro's taint work is cheap. With a 4500-second deadline and a 900-second query ceiling the run was stopped after 26 minutes with no case completed. Splitting also makes an unrecoverable batch slower to abandon, since each part burns its own share of the ceiling. - _Correction 2026-09-20: the malformed-CPG bug is fixed and was never what these runs were hitting._ 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 is the version the image has pinned since before any measurement here. Verified today on the real trigger: 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. So what remains under this task is a RUNTIME failure and must not be re-diagnosed as the graph bug. - _Quantified 2026-09-20, and it is a runtime failure._ A taint batch of 2,526 questions over the 673-file checkout does not complete in thirty minutes. Raising the query ceiling six-fold, from 300 to 1,800 seconds, changed nothing but how long the failure took: query time went from 363 s to 1,874 s and zero questions completed in both. The eight-way split retry did not rescue it, finishing its parts in about three minutes against a deadline with 3,300 seconds still on it, which is worth its own look. Joern 4.0.625 and 4.0.631 are indistinguishable here, 627.7 s against 619.1 s. The engine pin stays at 4.0.625: the newer release was built and measured and is equivalent on this subject, so there is no benefit to weigh against re-verifying every JavaScript and Python measurement. Evidence: `benchmarks/measurements/2026-09-20-php-runtime-after-graph-fix.json`. + - _Corrected 2026-09-20: PHP is not silent, and this project's own instrument is why it looked silent._ The census harness ordered its verdict completion-first, so a family with eighteen findings and no completed question reported "asked but no question completed" — and only the 500-region runs, the ones that time out, had been looked at. Fixed: findings are checked first, because a question-outcome count is bookkeeping and cannot hide the thing being censused. NodeGoat and VAmPI were never affected, since they complete every question. + - _What PHP actually detects on PMPro._ At 25 regions, 19 findings in seven minutes with no timeout; at 100 regions, 43 findings in twelve minutes with no timeout. Among them a true positive at `adminpages/discountcodes.php:749`: `sanitize_text_field($_REQUEST["s"])` reaching `$wpdb->get_results($sqlQuery, OBJECT)` through `$sqlQuery .= "WHERE code LIKE '%$s%' "` with no `prepare`. That is the class of CVE-2023-23488, which `php.toml`'s own comment names as the reason those WordPress sanitizers are excluded — the prediction and the measurement agree. + - _And a measured precision cost beside it._ Output encoding reports eighteen findings at 25 regions and the three sampled are all properly escaped, two with `esc_attr` and one coerced with `intval`. Those escapers are deliberately absent from a sanitizer list that is flat across families, because none of them stops SQL injection; keeping the injection finding above costs this family its precision, and that is the documented tension's price. + - _The scale limit, quantified._ Roughly one to two seconds per question. 25 and 100 regions complete cleanly; 500 regions asks 2,526 questions, needs more than the 1,800-second ceiling and is killed. Nothing pathological, and the retry's fast failure is still unexplained. + - _What is NOT the problem, each measured rather than assumed:_ the malformed-CPG bug (fixed in 4.0.625, verified on the real trigger file); the dataflow overlay (42 s on the full 637-file graph); the engine version (4.0.625 and 4.0.631 indistinguishable); and the bundled Stripe, Braintree and Twocheckout SDKs under `includes/lib`, which the vendored list does not cover — that was the hypothesis and the overlay costs 42 s with them and 37 s without, so it is refuted. + - **The remaining real gap is bookkeeping, not detection:** `questions_completed` is zero in every PHP run including the ones that find things, with `cross_partition_semantics_unresolved` and `partition_file_census_incomplete` present throughout. Findings and question outcomes disagree in public, and the admission policy reads the outcomes. Group 12 raised completion for JavaScript and Python and PHP was never measured. Evidence: `benchmarks/measurements/2026-09-20-php-detection-corrected.json`. - **Blocker still unidentified; investigation paused 2026-09-19 to return to vulnerability work.** Six hypotheses were measured and eliminated: batch size (17,700 requests answer in 44 s with a 2.6 MB response), dataflow cost (50 execution requests in 24 s), call depth 3 with parameter sources (24.7 s for 20), argument length (a 16 KB hook table against a 128 KB cap), hook callback resolution (1 s difference with 233 hooks), and build or sharding (79 s, no shards). Preparation measures 273.5 s. Every reproduction of the failing 22,125-request batch outside the push path answers in under a minute, and the difference from the production invocation is not known. The exception has never been read, because the backend logged a stack trace's tail and the corrected extractor reports its head, which on this engine is JVM startup noise. Evidence: `benchmarks/measurements/2026-09-19-php-blocker-open.json`. - _Boundary: Evaluation harness, full-run integration_ - _Depends: 13.1_ diff --git a/benchmarks/measurements/2026-09-20-php-detection-corrected.json b/benchmarks/measurements/2026-09-20-php-detection-corrected.json new file mode 100644 index 0000000..a0ca7bd --- /dev/null +++ b/benchmarks/measurements/2026-09-20-php-detection-corrected.json @@ -0,0 +1,134 @@ +{ + "schema_version": 1, + "experiment": "php-detection-corrected-v1", + "date": "2026-09-20", + "task": "pre-push-safety-net 13.2", + "claim_scope": "What PHP actually detects on Paid Memberships Pro, and why this project reported that it detected nothing.", + "the_correction": "PHP is not silent. It establishes findings on PMPro, including a true-positive SQL injection. Two separate mistakes made it look silent: the census harness checked question COMPLETION before findings, so a family with 18 findings and no completed question reported 'asked but no question completed'; and only the 500-region runs were looked at, which are the ones that time out.", + "what_was_wrong_with_the_instrument": { + "defect": "benchmarks/push/family_census.py ordered its verdict completion-first", + "effect": "every PHP finding was reported as 'asked but no question completed'", + "fix": "findings are checked first; a question-outcome count is bookkeeping and cannot hide the thing being censused", + "scope": "PHP only. NodeGoat and VAmPI complete every question, so their verdicts reached the findings check and were correct." + }, + "the_true_positive": { + "site": "adminpages/discountcodes.php:749", + "rung": "model_entailed", + "witness": "sanitize_text_field($_REQUEST[\"s\"]) -> $wpdb->get_results($sqlQuery, OBJECT), 9 steps", + "source_shape": "$sqlQuery .= \"WHERE code LIKE '%$s%' \"; then $wpdb->get_results($sqlQuery, OBJECT)", + "why_it_is_real": "A request value is interpolated into SQL with no $wpdb->prepare. sanitize_text_field strips tags and encodes; it does not escape SQL quotes. This is the class of CVE-2023-23488, which the PHP fact table's own comment names as the reason those WordPress sanitizers are excluded from the sanitizer list. The prediction and the measurement agree." + }, + "the_false_positives": { + "family": "output_encoding", + "count_at_25_regions": 18, + "sampled": [ + { + "site": "adminpages/admin_header.php:169", + "source": "echo 'pmpro_admin-' . esc_attr( $view );", + "verdict": "escaped, false positive" + }, + { + "site": "adminpages/discountcodes.php:770", + "source": "echo esc_attr( wp_unslash( $s ) );", + "verdict": "escaped, false positive" + }, + { + "site": "adminpages/admin_header.php:197", + "source": "echo admin_url( '...' . $specific_notification );", + "verdict": "the value was intval()'d upstream, false positive" + } + ], + "cause": "WordPress's esc_attr, esc_html and esc_url are deliberately absent from the PHP sanitizer list, because the list is flat across families and none of them stops SQL injection. Excluding them keeps the injection finding above and costs this family its precision. The tension is documented in php.toml; this is its measured price." + }, + "the_scale_limit": { + "runs": { + "25 regions": { + "questions": 127, + "questions_completed": 0, + "findings": 19, + "by_family": { + "injection": 1, + "output_encoding": 18 + }, + "query_seconds": 240.85, + "build_seconds": 122.08, + "timed_out": false, + "degradations": [ + "cross_partition_semantics_unresolved", + "partition_file_census_incomplete" + ] + }, + "100 regions": { + "questions": 520, + "questions_completed": 0, + "findings": 43, + "by_family": { + "injection": 10, + "output_encoding": 32, + "path": 1 + }, + "query_seconds": 522.27, + "build_seconds": 131.07, + "timed_out": false, + "degradations": [ + "cross_partition_semantics_unresolved", + "partition_file_census_incomplete" + ] + }, + "500 regions, 300s ceiling": { + "questions": 2526, + "questions_completed": 0, + "findings": 0, + "by_family": {}, + "query_seconds": 363.41, + "build_seconds": 144.04, + "timed_out": true, + "degradations": [ + "cross_partition_semantics_unresolved", + "partition_file_census_incomplete", + "process_timeout", + "query_failed", + "scratch_cleanup_incomplete", + "shard_query_incomplete" + ] + }, + "500 regions, 1800s ceiling": { + "questions": 2526, + "questions_completed": 0, + "findings": 0, + "by_family": {}, + "query_seconds": 1874.15, + "build_seconds": 113.24, + "timed_out": true, + "degradations": [ + "cross_partition_semantics_unresolved", + "partition_file_census_incomplete", + "process_timeout", + "query_failed", + "scratch_cleanup_incomplete", + "shard_query_incomplete" + ] + } + }, + "reading": "Cost is roughly one to two seconds per question and the failure is a straightforward scale limit. 25 and 100 regions complete with no timeout at all; 500 regions asks 2,526 questions, which at that rate needs more than the 1,800-second ceiling and gets killed. Nothing pathological." + }, + "the_remaining_real_gap": { + "symptom": "questions_completed is 0 in EVERY PHP run, including the ones that find things", + "always_present": [ + "cross_partition_semantics_unresolved", + "partition_file_census_incomplete" + ], + "reading": "Findings and question outcomes disagree in public: the family establishes a defect while every question it asked is recorded unresolved. Group 12 raised completion for JavaScript and Python and PHP was never measured. This is bookkeeping, not detection, and it is what the admission policy reads." + }, + "what_is_not_the_problem": { + "the_malformed_cpg_bug": "joernio/joern#6281, fixed in 4.0.625 which the image pins; verified on the real trigger file", + "the_dataflow_overlay": "42 seconds on the full 637-file graph, succeeds; 37 seconds on the 208-file tree without the bundled payment SDKs", + "the_engine_version": "4.0.625 and 4.0.631 are indistinguishable on this subject", + "vendored_libraries": "Two thirds of PMPro's PHP files are bundled Stripe, Braintree and Twocheckout SDKs under includes/lib, which the PHP vendored list does not cover. That was my hypothesis for the cost and the measurement refuted it: the overlay costs 42 s with them and 37 s without." + }, + "limits": [ + "One PHP subject, and its findings are unreviewed beyond the four witnesses inspected by hand.", + "The 18 output-encoding findings were sampled at three, not audited in full.", + "Region budgets of 25 and 100 are not the production population; they bound the question, not the repository." + ] +} diff --git a/benchmarks/push/family_census.py b/benchmarks/push/family_census.py index f66b5bb..64637cc 100644 --- a/benchmarks/push/family_census.py +++ b/benchmarks/push/family_census.py @@ -85,6 +85,10 @@ def main() -> int: "degradations": [str(d.get("reason")) for d in result.degradations], "questions_total": len(result.question_outcomes), "questions_completed": sum(1 for q in result.question_outcomes if q.status == "completed"), + "findings_total": len(result.findings), + # Why the outcomes say what they say, counted rather than inferred: a family can find something while + # every one of its questions is recorded unresolved, and the two numbers then disagree in public. + "outcome_status": dict(collections.Counter(q.status for q in result.question_outcomes)), } asked = collections.Counter(q.identity.family for q in result.question_outcomes) @@ -111,17 +115,24 @@ def main() -> int: "questions_completed": completed.get(family, 0), "findings": len(found.get(family, [])), "rungs": dict(collections.Counter(item["rung"] for item in found.get(family, []))), - "examples": found.get(family, [])[:3], + "examples": found.get(family, [])[:10], # The distinction that matters: a family that answered and found nothing is a different # statement from one that could not answer, and neither is "clean". + # + # Findings are checked FIRST, and that order is the whole point. It used to ask about completion + # before findings, so a family that established 18 findings on a PHP plugin while no question + # outcome was marked completed reported "asked but no question completed" -- and a census whose + # job is to say what each family finds reported nothing for a repository that had found a real + # SQL injection. A question-outcome count is a statement about bookkeeping; a finding is the + # thing being censused, and it cannot be hidden behind the other. "verdict": ( - "region offered, no question asked" + "establishes findings" + if found.get(family) + else "region offered, no question asked" if not asked.get(family) else "asked but no question completed" if not completed.get(family) else "answered, nothing found" - if not found.get(family) - else "establishes findings" ), } for family in families