diff --git a/src/openultrasast/cpg/queries/config.sc b/src/openultrasast/cpg/queries/config.sc index 70b517a..3b00c97 100644 --- a/src/openultrasast/cpg/queries/config.sc +++ b/src/openultrasast/cpg/queries/config.sc @@ -53,7 +53,8 @@ } val calls = { - val all = cpg.call.filter(c => matches(c.name, c.code)).l + // CPS-IT/mailqueue Extension::KEY has a null .method; exclude methodless calls before attribution. + val all = cpg.call.filter(c => Option(c.method).nonEmpty && matches(c.name, c.code)).l val inFile = if (fileS.isEmpty) all else all.filter(_.method.filename.endsWith(fileS)) if (function.isEmpty) inFile else inFile.filter(_.method.name == function) } diff --git a/src/openultrasast/cpg/queries/dominance.sc b/src/openultrasast/cpg/queries/dominance.sc index 34fb0ef..23eb179 100644 --- a/src/openultrasast/cpg/queries/dominance.sc +++ b/src/openultrasast/cpg/queries/dominance.sc @@ -172,7 +172,8 @@ // repository this was measured on, and unscoped rows attributed the model's unguarded query to the // handler. The sibling comparison is a statement about a FILE contradicting itself, which is what the // rung's justification says, so the file is also the right scope for it. - val allOperations = cpg.call.filter(isOperation).l + // CPS-IT/mailqueue Extension::KEY has a null .method; exclude methodless calls before attribution. + val allOperations = cpg.call.filter(c => Option(c.method).nonEmpty).filter(isOperation).l val opCalls = if (fileS.isEmpty) allOperations else allOperations.filter(_.method.filename.endsWith(fileS)) // ---- The module boundary an application puts between its route and its query -------------------------- @@ -236,7 +237,7 @@ if (fileS.isEmpty) Nil else fileMethods(fileS).flatMap { m => - m.call.l.flatMap { via => + m.call.l.filter(c => Option(c.method).nonEmpty).flatMap { via => via.callee.l .filter(_.isExternal) .flatMap(stub => stubTarget(stub.fullName)) diff --git a/src/openultrasast/cpg/queries/taint.sc b/src/openultrasast/cpg/queries/taint.sc index 366e2fe..d7c155a 100644 --- a/src/openultrasast/cpg/queries/taint.sc +++ b/src/openultrasast/cpg/queries/taint.sc @@ -226,6 +226,7 @@ // module region exists to cover the code that is in NO function, so its membership is direct. val moduleScope = function == "" + // CPS-IT/mailqueue Extension::KEY is a class-level PHP call with a null .method; exclude it from scope. def nestedInLabeled(m: io.shiftleft.codepropertygraph.generated.nodes.Method): Boolean = if (moduleScope) m.name == function else @@ -496,7 +497,7 @@ case c: io.shiftleft.codepropertygraph.generated.nodes.Call if originAnchors.contains(c.name) => true case c: io.shiftleft.codepropertygraph.generated.nodes.Call if concatenations.contains(c.name) => c.argument.l.sortBy(_.argumentIndex).headOption.exists(a => fixesOrigin(a, depth + 1)) - case i: io.shiftleft.codepropertygraph.generated.nodes.Identifier => + case i: io.shiftleft.codepropertygraph.generated.nodes.Identifier if i.method != null => // A variable fixes the origin when EVERY assignment to it in the method does. val writes = i.method.ast.isCall.nameExact(ASSIGNMENT).l.filter(_.argument.l.sortBy(_.argumentIndex).headOption.exists { case t: io.shiftleft.codepropertygraph.generated.nodes.Identifier => t.name == i.name @@ -790,7 +791,7 @@ case _ => false } ternary || (end :: elements.filter(stored)).distinctBy(_.id).exists { - case cfg: io.shiftleft.codepropertygraph.generated.nodes.CfgNode => + case cfg: io.shiftleft.codepropertygraph.generated.nodes.CfgNode if cfg.method != null => val method = cfg.method method.ast.isControlStructure.controlStructureType("IF").l.exists { cs => cs.condition.l.headOption.exists { cond => @@ -898,7 +899,7 @@ def fieldSourceNodes = if (fieldSrc != "true") Iterator.empty else { - val reads = cpg.call.nameExact(FIELD_ACCESS).filter(c => inScope(c.method)).l.filterNot(writeTarget) + val reads = cpg.call.nameExact(FIELD_ACCESS).filter(c => Option(c.method).exists(inScope)).l.filterNot(writeTarget) if (reads.isEmpty) Iterator.empty else reads.filter(r => taintedPrefixes(r.code.trim, fileS, r.method) && !overwrittenBefore(r)).iterator } @@ -958,7 +959,7 @@ val framework: List[io.shiftleft.codepropertygraph.generated.nodes.CfgNode] = callback.ast.isCall.filter(isSourceCall).l val fields: List[io.shiftleft.codepropertygraph.generated.nodes.CfgNode] = - callback.ast.isCall.nameExact(FIELD_ACCESS).l.filter(r => taintedPrefixes(r.code.trim, callback.filename, r.method)) + callback.ast.isCall.nameExact(FIELD_ACCESS).l.filter(r => Option(r.method).exists(m => taintedPrefixes(r.code.trim, callback.filename, m))) framework ++ fields } @@ -1013,7 +1014,7 @@ def hookSourceNodes = if (hookTable.isEmpty || hookApply.isEmpty) Iterator.empty else { - val applies = cpg.call.filter(c => hookApply.contains(c.name)).filter(c => inScope(c.method)).l + val applies = cpg.call.filter(c => hookApply.contains(c.name)).filter(c => Option(c.method).exists(inScope)).l if (applies.isEmpty) Iterator.empty else { val keys = applies.flatMap { call => @@ -1024,7 +1025,7 @@ else cpg.call .nameExact(INDEX_ACCESS) - .filter(c => inScope(c.method)) + .filter(c => Option(c.method).exists(inScope)) .l .filter(access => keys.contains(keyOf(access))) // ...and actually derived from the filtered value, not merely sharing a key name with it. @@ -1108,7 +1109,7 @@ def sinkCalls = sinkCandidatesMemo .getOrElseUpdate(sinksS, cpg.call.filter(c => sinkNames.exists(n => sinkMatches(c, n))).l) - .filter(c => inScope(c.method)) + .filter(c => Option(c.method).exists(inScope)) // ---- stage one of the two-stage join WITHOUT dataflow, for evidence mode (flow-aware-ranking, phase 2) -- // @@ -1170,7 +1171,7 @@ ) def fieldCarriedKind: String = - strongest(cpg.call.nameExact(FIELD_ACCESS).filter(c => inScope(c.method)).l.map(r => prefixKind(r.code.trim, fedFields(r.method.filename)))) + strongest(cpg.call.nameExact(FIELD_ACCESS).filter(c => Option(c.method).exists(inScope)).l.map(r => prefixKind(r.code.trim, fedFields(r.method.filename)))) def callbackFedKind(callback: String): String = callbackFedMemo.getOrElseUpdate( @@ -1187,7 +1188,7 @@ def hookCarriedKind: String = if (hookTable.isEmpty || hookApply.isEmpty) "" else - strongest(cpg.call.filter(c => hookApply.contains(c.name)).filter(c => inScope(c.method)).l.flatMap { call => + strongest(cpg.call.filter(c => hookApply.contains(c.name)).filter(c => Option(c.method).exists(inScope)).l.flatMap { call => val hook = call.argument.l.headOption.map(a => unquote(a.code)).getOrElse("") hookTable.getOrElse(hook, Nil).map(callbackFedKind) }) @@ -1233,7 +1234,7 @@ else { val confirmed = cpg.call .nameExact(FIELD_ACCESS) - .filter(c => inScope(c.method)) + .filter(c => Option(c.method).exists(inScope)) .l .exists(r => prefixKind(r.code.trim, fedFields(r.method.filename)) == "parameter" && taintedPrefixes(r.code.trim, r.method.filename, r.method)) if (confirmed) "source" else "parameter" @@ -1277,9 +1278,9 @@ // Field carrying is deliberately same-file in the existing abstraction. Retain // its file context conservatively, never claim every method carries the value. val fieldFiles = if (fieldCarriedKind.isEmpty) Set.empty[String] else - cpg.call.nameExact(FIELD_ACCESS).filter(c => inScope(c.method)).l.map(_.method.filename).toSet + cpg.call.nameExact(FIELD_ACCESS).filter(c => Option(c.method).exists(inScope)).l.map(_.method.filename).toSet val callbacks = if (hookApply.isEmpty) Nil else cpg.call.filter(c => hookApply.contains(c.name)) - .filter(c => inScope(c.method)).l.flatMap { call => + .filter(c => Option(c.method).exists(inScope)).l.flatMap { call => val hook = call.argument.l.headOption.map(a => unquote(a.code)).getOrElse("") hookTable.getOrElse(hook, Nil).flatMap(name => cpg.method.nameExact(name).filterNot(_.isExternal).l) }.distinct @@ -1401,7 +1402,7 @@ def traced(flow: io.joern.dataflowengineoss.language.Path): ujson.Arr = ujson.Arr.from(spliced(flow.elements.l).map { node => val where = node match { - case c: io.shiftleft.codepropertygraph.generated.nodes.CfgNode => c.method.filename.split("/").last + ":" + c.method.name + case c: io.shiftleft.codepropertygraph.generated.nodes.CfgNode => Option(c.method).map(m => m.filename.split("/").last + ":" + m.name).getOrElse("?") case _ => "?" } s"$where:${node.lineNumber.getOrElse(-1)} [${node.label}] ${node.code.take(120)}" @@ -1436,7 +1437,7 @@ "sinkArg0Literal" -> arg0Literal, "bounded" -> bounds.nonEmpty, "bound" -> bounds.headOption.getOrElse("").take(120), - "inLabeledScope" -> (function.isEmpty || nestedInLabeled(sink.method) || reachableMethods.contains(sink.method.fullName)) + "inLabeledScope" -> (function.isEmpty || Option(sink.method).exists(nestedInLabeled) || reachableMethods.contains(sink.method.fullName)) ) if (traceS == "true") row("trace") = traced(shortest) row diff --git a/src/openultrasast/cpg/queries/vocabulary.sc b/src/openultrasast/cpg/queries/vocabulary.sc index 77fd498..f874005 100644 --- a/src/openultrasast/cpg/queries/vocabulary.sc +++ b/src/openultrasast/cpg/queries/vocabulary.sc @@ -51,11 +51,13 @@ val sourceMemo = scala.collection.mutable.Map.empty[String, Boolean] def nearSource(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean = if (sourcePatterns.isEmpty) false - else + // CPS-IT/mailqueue Extension::KEY has a null .method; it cannot be near a method source. + else Option(c.method).exists { m => sourceMemo.getOrElseUpdate( - c.method.fullName, - c.method.ast.isCall.exists(x => sourcePatterns.exists(p => boundedPattern(p).matcher(x.code).find())) + m.fullName, + m.ast.isCall.exists(x => sourcePatterns.exists(p => boundedPattern(p).matcher(x.code).find())) ) + } def isModelled(c: io.shiftleft.codepropertygraph.generated.nodes.Call): Boolean = { val callee = calleeText(c.code) diff --git a/tests/test_cpg_queries_compile.py b/tests/test_cpg_queries_compile.py index 7aa9120..b6d159c 100644 --- a/tests/test_cpg_queries_compile.py +++ b/tests/test_cpg_queries_compile.py @@ -20,6 +20,7 @@ host's pytest mounted in, which needs no network and installs nothing: from __future__ import annotations import json +import re import subprocess from pathlib import Path @@ -28,6 +29,18 @@ import pytest QUERIES = Path("src/openultrasast/cpg/queries") +@pytest.mark.parametrize("name", ["taint", "dominance", "config", "census", "overlay", "vocabulary"]) +def test_query_scope_checks_guard_nullable_methods(name: str) -> None: + """PHP class constants can have no method; one unsafe scope check aborts the batch.""" + script = QUERIES / f"{name}.sc" + source = script.read_text() + assert source, f"{script} is empty" + # Require Option(node.method).exists(predicate), keeping the null guard at the call site. + unsafe = re.compile(r"\b(?:inScope|nestedInLabeled)\s*\(\s*[\w.]+\s*\.\s*method\s*\)") + sites = [source.count("\n", 0, match.start()) + 1 for match in unsafe.finditer(source)] + assert not sites, f"{script}: unguarded method scope checks at lines {sites}" + + def _joern() -> str | None: import shutil