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

src/openultrasast/cpg/queries/taint.sc

@@ -144,9 +144,22 @@
   // whole text, `apply_filters("h", md5("x"), $_GET["c"])` and `sanitize_text_field($_POST["x"])` were
   // sources themselves, so a filter's return was request data whatever it returned. The access inside the
   // argument is a source in its own right; nothing that reaches a sink through it is lost.
+  // Of the operators, only an ACCESS names its own value: `$_GET["c"]`, `req.body.x`. A conditional or a
+  // concatenation whose text contains `$_GET` is an expression built from the access, not the access -- matched
+  // on its text, `isset($_GET["id"]) ? intval($_GET["id"]) : null` was itself a source, downstream of the cast.
+  val accessOperators = Set(
+    "<operator>.fieldAccess",
+    "<operator>.indexAccess",
+    "<operator>.indirectFieldAccess",
+    "<operator>.indirectIndexAccess"
+  )
+
   def isSourceCall(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean = {
-    val named = if (c.name.startsWith("<operator")) c.code else c.code.takeWhile(_ != '(')
-    sourcePatterns.exists(p => named.contains(p))
+    val named =
+      if (accessOperators.contains(c.name)) c.code
+      else if (c.name.startsWith("<operator")) ""
+      else c.code.takeWhile(_ != '(')
+    named.nonEmpty && sourcePatterns.exists(p => named.contains(p))
   }
 
   def frameworkSources = cpg.call.filter(isSourceCall)
@@ -570,7 +583,7 @@
                     case target: io.shiftleft.codepropertygraph.generated.nodes.Call if target.name == INDEX_ACCESS =>
                       val key = keyOf(target)
                       if (key.isEmpty) None
-                      else if (args(1).start.reachableByFlows(seeds.iterator).l.filter(plausible).exists(f => !f.elements.l.exists(sanitizesHere)))
+                      else if (args(1).start.reachableByFlows(seeds.iterator).l.filter(plausible).exists(f => !f.elements.l.exists(sanitizesHere) && !sanitizedBetween(f.elements.l)))
                         Some(key)
                       else None
                     case _ => None
@@ -697,6 +710,36 @@
     byName || sanitizerNames.exists(n => code.startsWith(n + "(") || code.contains("'" + n + "'") || code.contains("\"" + n + "\""))
   }
 
+  // A sanitizer the value passes THROUGH between two path elements. The engine can step from a sanitizer's
+  // argument straight to the expression around the call -- `$params["id"]` inside
+  // `isset($params["id"]) ? intval($params["id"]) : null` went to the conditional, and `intval` itself was
+  // never an element -- so checking elements alone reported PMPro's integer-cast REST level id as an SQL
+  // injection. Where the next element CONTAINS the previous one, the syntax between them is what the value
+  // was wrapped in; a sanitizer there sanitized it. A sanitizer elsewhere in the expression does not count.
+  def sanitizedBetween(elements: List[io.shiftleft.codepropertygraph.generated.nodes.AstNode]): Boolean =
+    elements.zip(elements.drop(1)).exists { case (inner, outer) =>
+      var node: Option[io.shiftleft.codepropertygraph.generated.nodes.AstNode] = inner._astIn.nextOption() match {
+        case Some(parent: io.shiftleft.codepropertygraph.generated.nodes.AstNode) => Some(parent)
+        case _                                                                     => None
+      }
+      var wrapped = false
+      var reached = false
+      var depth   = 0
+      while (node.isDefined && !reached && depth < 8) {
+        val current = node.get
+        if (current.id == outer.id) reached = true
+        else {
+          if (sanitizesHere(current)) wrapped = true
+          node = current._astIn.nextOption() match {
+            case Some(parent: io.shiftleft.codepropertygraph.generated.nodes.AstNode) => Some(parent)
+            case _                                                                     => None
+          }
+          depth += 1
+        }
+      }
+      reached && wrapped
+    }
+
   // The candidate sink calls for a FAMILY do not depend on the region, and scanning every call in the graph
   // for each of 2,487 requests was a large share of what made a request cost seconds -- in evidence mode,
   // which asks for no dataflow at all, it alone took a query past an hour. Scanned once per sink list and
@@ -1005,7 +1048,7 @@
     flows
       .map { flow =>
         val elements   = flow.elements.map(_.code).l
-        val sanitized  = sanitizerNames.nonEmpty && flow.elements.l.exists(node => sanitizesHere(node))
+        val sanitized  = sanitizerNames.nonEmpty && (flow.elements.l.exists(node => sanitizesHere(node)) || sanitizedBetween(flow.elements.l))
         val sourceKind = sourceKindOf(flow.elements.l.headOption)
         ((sourceKind, elements.headOption.getOrElse("").take(200), sanitized), (elements.size, flow))
       }

tests/test_cpg_queries_compile.py

@@ -858,3 +858,56 @@ def test_the_field_join_links_methods_only_through_the_object_itself(tmp_path: P
     flows = {rid: [r for r in payload[rid] if isinstance(r, dict) and r.get("sourceKind")] for rid in ("local", "object")}
     assert not flows["local"], f"another function's local variable tainted this parameter's field: {flows['local']}"
     assert flows["object"], "object state written in one method and read in another is no longer traced"
+
+
+@pytest.mark.skipif(_joern() is None, reason="joern is not installed on this machine")
+def test_a_sanitizer_the_value_passes_through_inside_an_expression_counts(tmp_path: Path) -> None:
+    """`isset($p["id"]) ? intval($p["id"]) : null`: the engine steps from intval's argument to the conditional,
+    so `intval` is never a path element. PMPro's REST level id, cast exactly so, was reported as an injection.
+    A sanitizer in the OTHER branch must not count."""
+    from openultrasast.cpg.backend import extract_payload
+
+    frontend = __import__("shutil").which("php2cpg")
+    if frontend is None:
+        pytest.skip("php2cpg is not installed")
+    src = tmp_path / "src"
+    src.mkdir()
+    (src / "app.php").write_text(
+        "<?php\n"
+        "function cast() { global $wpdb; $id = isset($_GET['id']) ? intval($_GET['id']) : null;\n"
+        '  return $wpdb->query("DELETE FROM t WHERE id = " . $id); }\n'
+        "function other_branch($x) { global $wpdb; $id = isset($_GET['id']) ? $_GET['id'] : intval($x);\n"
+        '  return $wpdb->query("DELETE FROM t WHERE id = " . $id); }\n'
+    )
+    cpg = tmp_path / "cpg.bin"
+    built = subprocess.run([frontend, str(src), "-o", str(cpg)], capture_output=True, text=True, timeout=600, check=False)
+    if not cpg.is_file():
+        pytest.skip(f"could not build a sample cpg: {(built.stderr or '')[-200:]}")
+    common = {"sources": "$_GET", "sinks": "$wpdb->query", "sanitizers": "intval", "file": "app.php"}
+    requests = tmp_path / "requests.json"
+    requests.write_text(json.dumps({"cast": {**common, "function": "cast"}, "other": {**common, "function": "other_branch"}}))
+    done = subprocess.run(
+        [
+            _joern() or "joern",
+            "--script",
+            str((QUERIES / "taint.sc").resolve()),
+            "--param",
+            f"cpgFile={cpg}",
+            "--param",
+            f"requestsFile={requests}",
+        ],
+        capture_output=True,
+        text=True,
+        timeout=900,
+        check=False,
+        cwd=str(tmp_path),
+    )
+    assert done.returncode == 0, (done.stderr or done.stdout or "")[-500:]
+    payload = extract_payload(done.stdout or "")
+    assert isinstance(payload, dict)
+    unsanitized = {
+        rid: [r for r in payload[rid] if isinstance(r, dict) and r.get("sourceKind") and not r.get("sanitized")]
+        for rid in ("cast", "other")
+    }
+    assert not unsanitized["cast"], f"a value cast inside a conditional was reported unsanitized: {unsanitized['cast']}"
+    assert unsanitized["other"], "a sanitizer in the other branch of the conditional cleaned the tainted branch"