Java: add test for spurious flow from path graph deduplication

2026-04-26 17:25:19 +02:00 · 2023-10-11 14:06:32 +02:00
parent 8efdc2df7b
commit 0eb543e0a9
3 changed files with 184 additions and 0 deletions
--- a/java/ql/test/library-tests/dataflow/deduplicate-path-graph/A.java
+++ b/java/ql/test/library-tests/dataflow/deduplicate-path-graph/A.java
@@ -0,0 +1,34 @@
+import java.util.function.Function;
+
+class A {
+  String source() { return ""; }
+
+  void sink(String s) { }
+
+  String propagateState(String s, String state) {
+    return "";
+  }
+
+  void foo() {
+    Function<String, String> lambda = Math.random() > 0.5
+      ? x -> propagateState(x, "A")
+      : x -> propagateState(x, "B");
+
+    String step0 = source();
+    String step1 = lambda.apply(step0);
+    String step2 = lambda.apply(step1);
+
+    sink(step2);
+  }
+
+  void bar() {
+    Function<String, String> lambda =
+      (x -> Math.random() > 0.5 ? propagateState(x, "A") : propagateState(x, "B"));
+
+    String step0 = source();
+    String step1 = lambda.apply(step0);
+    String step2 = lambda.apply(step1);
+
+    sink(step2);
+  }
+}
--- a/java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.expected
+++ b/java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.expected
@@ -0,0 +1,67 @@
+nodes
+| A.java:14:9:14:9 | x : String | semmle.label | x : String |
+| A.java:14:14:14:35 | propagateState(...) : String | semmle.label | propagateState(...) : String |
+| A.java:14:29:14:29 | x : String | semmle.label | x : String |
+| A.java:15:9:15:9 | x : String | semmle.label | x : String |
+| A.java:15:14:15:35 | propagateState(...) : String | semmle.label | propagateState(...) : String |
+| A.java:15:29:15:29 | x : String | semmle.label | x : String |
+| A.java:17:20:17:27 | source(...) : String | semmle.label | source(...) : String |
+| A.java:18:20:18:38 | apply(...) : String | semmle.label | apply(...) : String |
+| A.java:18:33:18:37 | step0 : String | semmle.label | step0 : String |
+| A.java:19:20:19:38 | apply(...) : String | semmle.label | apply(...) : String |
+| A.java:19:33:19:37 | step1 : String | semmle.label | step1 : String |
+| A.java:21:10:21:14 | step2 | semmle.label | step2 |
+| A.java:26:8:26:8 | x : String | semmle.label | x : String |
+| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : String |
+| A.java:26:35:26:56 | propagateState(...) : String | semmle.label | propagateState(...) : String |
+| A.java:26:50:26:50 | x : String | semmle.label | x : String |
+| A.java:26:60:26:81 | propagateState(...) : String | semmle.label | propagateState(...) : String |
+| A.java:26:75:26:75 | x : String | semmle.label | x : String |
+| A.java:28:20:28:27 | source(...) : String | semmle.label | source(...) : String |
+| A.java:29:20:29:38 | apply(...) : String | semmle.label | apply(...) : String |
+| A.java:29:33:29:37 | step0 : String | semmle.label | step0 : String |
+| A.java:30:20:30:38 | apply(...) : String | semmle.label | apply(...) : String |
+| A.java:30:33:30:37 | step1 : String | semmle.label | step1 : String |
+| A.java:32:10:32:14 | step2 | semmle.label | step2 |
+edges
+| A.java:14:9:14:9 | x : String | A.java:14:29:14:29 | x : String |
+| A.java:14:29:14:29 | x : String | A.java:14:14:14:35 | propagateState(...) : String |
+| A.java:15:9:15:9 | x : String | A.java:15:29:15:29 | x : String |
+| A.java:15:29:15:29 | x : String | A.java:15:14:15:35 | propagateState(...) : String |
+| A.java:17:20:17:27 | source(...) : String | A.java:18:33:18:37 | step0 : String |
+| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String |
+| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String |
+| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String |
+| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String |
+| A.java:19:20:19:38 | apply(...) : String | A.java:21:10:21:14 | step2 |
+| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String |
+| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String |
+| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String |
+| A.java:26:8:26:8 | x : String | A.java:26:50:26:50 | x : String |
+| A.java:26:8:26:8 | x : String | A.java:26:75:26:75 | x : String |
+| A.java:26:35:26:56 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String |
+| A.java:26:50:26:50 | x : String | A.java:26:35:26:56 | propagateState(...) : String |
+| A.java:26:60:26:81 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String |
+| A.java:26:75:26:75 | x : String | A.java:26:60:26:81 | propagateState(...) : String |
+| A.java:28:20:28:27 | source(...) : String | A.java:29:33:29:37 | step0 : String |
+| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String |
+| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String |
+| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String |
+| A.java:30:20:30:38 | apply(...) : String | A.java:32:10:32:14 | step2 |
+| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String |
+| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String |
+subpaths
+| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
+| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:18:20:18:38 | apply(...) : String |
+| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String | A.java:14:14:14:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
+| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String | A.java:15:14:15:35 | propagateState(...) : String | A.java:19:20:19:38 | apply(...) : String |
+| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:29:20:29:38 | apply(...) : String |
+| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | A.java:26:13:26:81 | ...?...:... : String | A.java:30:20:30:38 | apply(...) : String |
+spuriousFlow
+| A.java:14:14:14:35 | propagateState(...) : String | B | A |
+| A.java:15:14:15:35 | propagateState(...) : String | A | B |
+| A.java:26:35:26:56 | propagateState(...) : String | B | A |
+| A.java:26:60:26:81 | propagateState(...) : String | A | B |
+#select
+| A.java:17:20:17:27 | source(...) : String | A.java:17:20:17:27 | source(...) : String | A.java:21:10:21:14 | step2 | Flow |
+| A.java:28:20:28:27 | source(...) : String | A.java:28:20:28:27 | source(...) : String | A.java:32:10:32:14 | step2 | Flow |
--- a/java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.ql
+++ b/java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.ql
@@ -0,0 +1,83 @@
+/**
+ * @kind path-problem
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import DataFlow
+
+MethodAccess propagateCall(string state) {
+  result.getMethod().getName() = "propagateState" and
+  state = result.getArgument(1).(StringLiteral).getValue()
+}
+
+module TestConfig implements StateConfigSig {
+  class FlowState = string;
+
+  predicate isSource(Node n, FlowState state) {
+    n.asExpr().(MethodAccess).getMethod().getName() = "source" and state = ["A", "B"]
+  }
+
+  predicate isSink(Node n, FlowState state) {
+    n.asExpr() = any(MethodAccess acc | acc.getMethod().getName() = "sink").getAnArgument() and
+    state = ["A", "B"]
+  }
+
+  predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) {
+    exists(MethodAccess call |
+      call = propagateCall(state1) and
+      state2 = state1 and
+      node1.asExpr() = call.getArgument(0) and
+      node2.asExpr() = call
+    )
+  }
+}
+
+module TestFlow = DataFlow::GlobalWithState<TestConfig>;
+
+module Graph = DataFlow::DeduplicatePathGraph<TestFlow::PathNode, TestFlow::PathGraph>;
+
+/**
+ * Holds if `node` is reachable from a call to `propagateState` with the given `state` argument.
+ * `call` indicates if a call step was taken (i.e. into a subpath).
+ *
+ * We use this to check if one `propagateState` can flow out of another, which is not allowed.
+ */
+predicate reachableFromPropagate(Graph::PathNode node, string state, boolean call) {
+  node.getNode().asExpr() = propagateCall(state) and call = false
+  or
+  exists(Graph::PathNode prev | reachableFromPropagate(prev, state, call) |
+    Graph::edges(prev, node)
+    or
+    Graph::subpaths(prev, _, _, node) // arg -> out
+  )
+  or
+  exists(Graph::PathNode prev |
+    reachableFromPropagate(prev, state, _) and
+    Graph::subpaths(prev, node, _, _) and // arg -> parameter
+    call = true
+  )
+  or
+  exists(Graph::PathNode prev |
+    reachableFromPropagate(prev, state, call) and
+    Graph::subpaths(_, _, prev, node) and // return -> out
+    call = false
+  )
+}
+
+/**
+ * Holds if `node` is the return value of a `propagateState` call that appears to be reachable
+ * with a different state than the one propagated by the call, indicating spurious flow resulting from
+ * merging path nodes.
+ */
+query predicate spuriousFlow(Graph::PathNode node, string state1, string state2) {
+  reachableFromPropagate(node, state1, _) and
+  node.getNode().asExpr() = propagateCall(state2) and
+  state1 != state2
+}
+
+import Graph::PathGraph
+
+from Graph::PathNode source, Graph::PathNode sink
+where TestFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
+select source, source, sink, "Flow"