diff --git a/benchmarks/repos/wpstatistics.toml b/benchmarks/repos/wpstatistics.toml new file mode 100644 index 0000000..8718a83 --- /dev/null +++ b/benchmarks/repos/wpstatistics.toml @@ -0,0 +1,48 @@ +# WP Statistics 13.1.5 -- a WordPress plugin with an unauthenticated SQL injection reached through a hook. +# +# The third WordPress checkout, and the one chosen because it is NOT expected to be found. PMPro's CVE is a +# concatenation inside the function that receives the value; this one crosses WordPress's filter dispatcher, +# which is the shape most plugin data actually travels in. A table that only lists hits teaches nothing about +# where the engine stops. +# +# Verified in the code. At this commit `includes/class-wp-statistics-pages.php:225` reads: +# +# $exist = $wpdb->get_row("SELECT `page_id` FROM ... AND `id` = {$current_page['id']}", ARRAY_A); +# +# and one line below, the SAME value is used correctly: +# +# $wpdb->query($wpdb->prepare("UPDATE ... AND `id` = %d", $current_page['id'])); +# +# The value arrives from an unauthenticated REST hit. `Hits::rest_params()` copies every `$_REQUEST` key into +# `$this->rest_hits`; `Hits::set_current_page()` returns `'id' => $this->rest_hits->current_page_id`; and it +# is bound to the sink not by a call but by +# +# 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 of that edge name the callback with a STRING, so no frontend can draw it. The fix (bc98b414) +# wraps both values in `esc_sql`, which is a modelled sanitizer -- so if the flow were visible at all, the +# pair would separate. + +name = "wpstatistics" +url = "https://github.com/wp-statistics/wp-statistics.git" +commit = "ca66e7b89587dce3ee189f1eb42211ad111db4b7" +language = "php" +license = "GPL-2.0" +license_file = "license.txt" +measures = ["detection"] +note = """ +The parent of the fix commit bc98b414 ("Sanitize current_page_id & current_page_type in rest request", +2022-02-13), released in the 13.1.x line. Nothing is vendored -- the recipe fetches on request and CI never +does. +""" + +[[known]] +id = "CVE-2022-25148" +family = "injection" +weakness = "unauthenticated SQL injection: current_page_id from an unauthenticated REST hit is interpolated into a $wpdb->get_row query, having crossed the wp_statistics_current_page filter" +file = "includes/class-wp-statistics-pages.php" +function = "record" +line = 225 +reference = "fixed in bc98b4146afd6f4b29fda1136772dcf1a42f26e6, which applies esc_sql to current_page_type and current_page_id at their source; the vulnerable line is verified in the checkout" +in_scope = true diff --git a/src/openultrasast/cpg/backend.py b/src/openultrasast/cpg/backend.py index 00e629e..97250da 100644 --- a/src/openultrasast/cpg/backend.py +++ b/src/openultrasast/cpg/backend.py @@ -22,6 +22,7 @@ from __future__ import annotations import json import logging import os +import re import shutil import subprocess import tempfile @@ -94,6 +95,11 @@ class CpgResult: cpg_path: Path run: Callable[[str, Mapping[str, object]], object | None] run_batch: Callable[[str, Mapping[str, Mapping[str, object]]], dict[str, list[object]] | None] | None = None + # Files the frontend read and could not turn into a graph. A CPG is not all-or-nothing: `php2cpg` logs a + # warning per file it drops and still exits 0 with a graph that is missing them, so a scan over the + # remainder is a scan of LESS CODE THAN IT WAS ASKED ABOUT, and silence about those files is the same + # quiet failure as an empty query result. The names travel with the CPG so the driver can say so. + unparsed: tuple[str, ...] = () class CpgBackend(Protocol): @@ -144,33 +150,45 @@ class JoernBackend: cpg_path = scratch / "cpg.bin" command = [parse or "joern-parse", self._heap_flag(), str(root), "--output", str(cpg_path)] completed = self._run(command, timeout=self.build_timeout, cwd=scratch) + unparsed = _unparsed_files(completed) if completed is None or completed.returncode != 0 or not cpg_path.is_file(): detail = (completed.stderr or completed.stdout or "")[-400:] if completed is not None else "timeout" logger.warning("cpg build failed for %s: %s", root, detail) - if not self._build_with_frontend(root, cpg_path, scratch, language): + retried = self._build_with_frontend(root, cpg_path, scratch, language) + if retried is None: return None + unparsed = retried + if unparsed: + logger.warning("the frontend could not parse %d file(s) under %s: %s", len(unparsed), root, ", ".join(unparsed[:5])) return CpgResult( cpg_path=cpg_path, run=lambda query, params: self.query(cpg_path, query, params), run_batch=lambda query, requests: self.query_batch(cpg_path, query, requests), + unparsed=unparsed, ) - def _build_with_frontend(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> bool: - """Retry the build through the language frontend alone. False when there is nothing to retry with.""" + def _build_with_frontend(self, root: Path, cpg_path: Path, scratch: Path, language: str) -> tuple[str, ...] | None: + """Retry the build through the language frontend alone. + + Returns the files the frontend dropped -- possibly none -- or ``None`` when there is nothing to retry + with or the retry failed too. An empty tuple and ``None`` are different answers here for the same + reason they are everywhere else in this module: one means "built, and complete", the other means + "could not build". + """ frontend = _FRONTENDS.get(language.lower()) if not frontend: - return False + return None binary = shutil.which(frontend) if binary is None: logger.warning("no frontend %s on PATH to retry the build for %s", frontend, root) - return False + return None completed = self._run([binary, self._heap_flag(), str(root), "-o", str(cpg_path)], timeout=self.build_timeout, cwd=scratch) if completed is None or completed.returncode != 0 or not cpg_path.is_file(): detail = (completed.stderr or completed.stdout or "")[-400:] if completed is not None else "timeout" logger.warning("%s also failed for %s: %s", frontend, root, detail) - return False + return None logger.info("built the cpg for %s with %s after joern-parse failed", root, frontend) - return True + return _unparsed_files(completed) def query_batch(self, cpg_path: Path, query: str, requests: Mapping[str, Mapping[str, object]]) -> dict[str, list[object]] | None: """Run ONE script invocation carrying many requests, keyed back to their ids. @@ -261,6 +279,41 @@ class JoernBackend: return None +# Every Joern frontend logs a dropped file the same way, at WARN, and then carries on: +# +# WARN AstCreationPass Failed to process '/abs/path/to/file.php' +# +# The quotes are part of the line, which is what makes this safely greppable rather than a guess at column +# positions. Several passes report the same file, so the caller de-duplicates. +_UNPARSED = re.compile(r"Failed to process '([^']+)'") + + +def _unparsed_files(completed: subprocess.CompletedProcess[str] | None) -> tuple[str, ...]: + """The files the frontend read and dropped, in first-seen order. + + This exists because a Joern frontend fails a file WITHOUT failing the build. `php2cpg` 4.0.623 feeds + several files to one `php-parse` process and reads its stdout and stderr merged; php-parse writes a + `====> File :` banner to stderr the moment it starts the next file, while the previous file's JSON + -- megabytes of it -- is still draining from a block-buffered stdout. The banner lands inside the JSON, + ujson reports `expected json value got "="`, and the frontend drops that file. Reproducibly, on real + plugin code: + + one 1834-line file alone -> a 142KB CPG, no failures + the same file plus a two-line file -> a 5.7KB CPG, BOTH files dropped, exit status 0 + + So the size that matters is one file's AST dump, not the repository's, and one oversized file can empty + the whole graph. Before this, that arrived as a scan reporting no findings and no degradations -- a clean + bill of health over a graph containing nothing at all. + """ + if completed is None: + return () + seen: dict[str, None] = {} + for stream in (completed.stdout or "", completed.stderr or ""): + for match in _UNPARSED.finditer(stream): + seen.setdefault(match.group(1), None) + return tuple(seen) + + def _render(value: object) -> str: """CPGQL parameters are strings; a sequence becomes a comma-separated list the script splits.""" if isinstance(value, str): diff --git a/src/openultrasast/cpg/queries/config.sc b/src/openultrasast/cpg/queries/config.sc index 5941afe..44a1a7b 100644 --- a/src/openultrasast/cpg/queries/config.sc +++ b/src/openultrasast/cpg/queries/config.sc @@ -73,7 +73,13 @@ def field(name: String): String = req.obj.get(name).map(_.str).getOrElse("") id -> ujson.Arr(rowsFor(field("settings"), field("function"), field("file")): _*) } - println(ujson.write(ujson.Obj.from(answers))) + // A census of the graph, under a key no request id can collide with (ids are numbers). A frontend can + // fail every file and STILL exit 0 with a valid, empty CPG -- `joern-parse` does not even propagate the + // per-file warnings -- so "no rows" and "no graph" are indistinguishable to the driver without this. + // It rides the batch rather than costing its own invocation, because JVM startup is what this whole + // batching design exists to avoid. + val census = ujson.Arr(ujson.Obj("methods" -> cpg.method.size.toString, "files" -> cpg.file.size.toString)) + println(ujson.write(ujson.Obj.from(answers.toSeq :+ ("__census__" -> census)))) } else { println(ujson.write(ujson.Arr(rowsFor(settings, function, file): _*))) } diff --git a/src/openultrasast/cpg/queries/dominance.sc b/src/openultrasast/cpg/queries/dominance.sc index dee665e..250dfe1 100644 --- a/src/openultrasast/cpg/queries/dominance.sc +++ b/src/openultrasast/cpg/queries/dominance.sc @@ -185,7 +185,13 @@ ): _* ) } - println(ujson.write(ujson.Obj.from(answers))) + // A census of the graph, under a key no request id can collide with (ids are numbers). A frontend can + // fail every file and STILL exit 0 with a valid, empty CPG -- `joern-parse` does not even propagate the + // per-file warnings -- so "no rows" and "no graph" are indistinguishable to the driver without this. + // It rides the batch rather than costing its own invocation, because JVM startup is what this whole + // batching design exists to avoid. + val census = ujson.Arr(ujson.Obj("methods" -> cpg.method.size.toString, "files" -> cpg.file.size.toString)) + println(ujson.write(ujson.Obj.from(answers.toSeq :+ ("__census__" -> census)))) } else { println(ujson.write(ujson.Arr(rowsFor(operations, dischargers, function, file, operationRequires, dischargersByKind): _*))) } diff --git a/src/openultrasast/cpg/queries/taint.sc b/src/openultrasast/cpg/queries/taint.sc index 3bdb956..44cd18c 100644 --- a/src/openultrasast/cpg/queries/taint.sc +++ b/src/openultrasast/cpg/queries/taint.sc @@ -280,7 +280,13 @@ ): _* ) } - println(ujson.write(ujson.Obj.from(answers))) + // A census of the graph, under a key no request id can collide with (ids are numbers). A frontend can + // fail every file and STILL exit 0 with a valid, empty CPG -- `joern-parse` does not even propagate the + // per-file warnings -- so "no rows" and "no graph" are indistinguishable to the driver without this. + // It rides the batch rather than costing its own invocation, because JVM startup is what this whole + // batching design exists to avoid. + val census = ujson.Arr(ujson.Obj("methods" -> cpg.method.size.toString, "files" -> cpg.file.size.toString)) + println(ujson.write(ujson.Obj.from(answers.toSeq :+ ("__census__" -> census)))) } else { println( ujson.write(ujson.Arr(rowsFor(sources, sinks, sanitizers, function, parameterSources, file, callDepth, boundedSinks): _*)) diff --git a/src/openultrasast/model/scan.py b/src/openultrasast/model/scan.py index 2d2885d..8e00c32 100644 --- a/src/openultrasast/model/scan.py +++ b/src/openultrasast/model/scan.py @@ -126,6 +126,16 @@ def scan_repository( degradations=({"stage": "model", "reason": "cpg_build_failed"},), ) + # A CPG the frontend built with files missing is not the repository the caller asked about, and no + # verdict over it can say anything about the code that was dropped. There is no rung for that, because + # the ladder labels verdicts and none was ever produced here -- so it is a degradation, named, with the + # files listed. See `cpg.backend._unparsed_files` for how a frontend drops a file while exiting 0. + if getattr(cpg, "unparsed", ()): # a backend need not carry the field + unparsed = tuple(cpg.unparsed) + degradations.append( + {"stage": "model", "reason": "files_unparsed", "count": len(unparsed), "files": [Path(name).name for name in unparsed[:20]]} + ) + # Count calls here rather than reading a client's own meter: the budget is this driver's contract and # must hold for any client, including one that keeps no usage log. counted = _CountingClient(client) if client is not None else None @@ -150,6 +160,7 @@ def scan_repository( # the queries themselves are milliseconds once the CPG is loaded. rows_by_id: dict[str, list[object]] = {} per_kind: dict[str, float] = {} + census_reported = False query_started = time.monotonic() for kind, requests in _grouped(work).items(): kind_started = time.monotonic() @@ -162,6 +173,18 @@ def scan_repository( # failed at CPG load and the scan reported 500 regions examined with nothing found. degradations.append({"stage": "model", "reason": "query_failed", "kind": kind, "requests": len(requests)}) else: + # The graph census rides the batch under a reserved key. A frontend that fails every file + # still exits 0 with a valid CPG containing nothing, and `joern-parse` does not propagate + # its warnings, so without this a scan of an EMPTY graph is indistinguishable from a scan + # that found nothing -- which is how a two-file WordPress slice reported a clean bill of + # health over a 12KB graph with no methods in it at all. + census = answered_batch.pop("__census__", None) + if census and not census_reported: + entry = census[0] if isinstance(census[0], Mapping) else {} + methods = int(str(entry.get("methods", "0")) or 0) + if methods == 0: + degradations.append({"stage": "model", "reason": "cpg_empty", "files": int(str(entry.get("files", "0")) or 0)}) + census_reported = True rows_by_id.update(answered_batch) else: # a backend without batching still works, one call at a time for rid, params in requests.items(): diff --git a/tests/test_cpg_backend.py b/tests/test_cpg_backend.py index 6f94720..22a6586 100644 --- a/tests/test_cpg_backend.py +++ b/tests/test_cpg_backend.py @@ -8,6 +8,7 @@ Nothing here imports a JVM binding, and every test runs with Joern absent. from __future__ import annotations import json +import subprocess from pathlib import Path import pytest @@ -341,3 +342,29 @@ def test_a_batch_goes_through_a_file_not_the_command_line(tmp_path: Path) -> Non assert staged, "the batch is staged to a file" written = Path(staged[0].split("=", 1)[1]) assert written.is_file() and len(written.read_text()) > 200_000, "and the file holds the whole payload" + + +def test_a_file_the_frontend_dropped_is_carried_off_the_build() -> None: + """A Joern frontend fails a file without failing the build, and exits 0 with the rest of the graph. + + php2cpg 4.0.623 does exactly this: one oversized file corrupts the batched parser stream, that file is + logged at WARN and dropped, and the CPG comes back missing it. A scan over the remainder that says + nothing is a clean bill of health over code nobody looked at. + """ + warning = ( + "2026-09-09 17:46:21.892 WARN AstCreationPass Failed to process '/src/class.memberorder.php'\n" + "2026-09-09 17:46:21.892 WARN SymbolSummaryPass Failed to process '/src/class.memberorder.php'\n" + "2026-09-09 17:46:21.893 WARN AstCreationPass Failed to process '/src/rest-api.php'\n" + ) + from openultrasast.cpg.backend import _unparsed_files + + completed = subprocess.CompletedProcess(args=[], returncode=0, stdout="", stderr=warning) + assert _unparsed_files(completed) == ("/src/class.memberorder.php", "/src/rest-api.php") + + +def test_a_build_with_nothing_dropped_reports_no_unparsed_files() -> None: + from openultrasast.cpg.backend import _unparsed_files + + completed = subprocess.CompletedProcess(args=[], returncode=0, stdout="Successfully wrote graph", stderr="") + assert _unparsed_files(completed) == () + assert _unparsed_files(None) == ()