Shared: Add DataFlow::DeduplicatePathGraph

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);
+  }
+}

java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.expected

@@ -0,0 +1,79 @@
+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: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: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:8:26:8 | x : String | semmle.label | x : String |
+| A.java:26:13:26:81 | ...?...:... : String | semmle.label | ...?...:... : 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: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: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 | provenance |  |
+| A.java:14:29:14:29 | x : String | A.java:14:14:14:35 | propagateState(...) : String | provenance | Config |
+| A.java:15:9:15:9 | x : String | A.java:15:29:15:29 | x : String | provenance |  |
+| A.java:15:29:15:29 | x : String | A.java:15:14:15:35 | propagateState(...) : String | provenance | Config |
+| A.java:17:20:17:27 | source(...) : String | A.java:18:33:18:37 | step0 : String | provenance |  |
+| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String | provenance |  |
+| A.java:18:20:18:38 | apply(...) : String | A.java:19:33:19:37 | step1 : String | provenance |  |
+| A.java:18:33:18:37 | step0 : String | A.java:14:9:14:9 | x : String | provenance |  |
+| A.java:18:33:18:37 | step0 : String | A.java:15:9:15:9 | x : String | provenance |  |
+| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String | provenance | Config |
+| A.java:18:33:18:37 | step0 : String | A.java:18:20:18:38 | apply(...) : String | provenance | Config |
+| A.java:19:20:19:38 | apply(...) : String | A.java:21:10:21:14 | step2 | provenance |  |
+| A.java:19:33:19:37 | step1 : String | A.java:14:9:14:9 | x : String | provenance |  |
+| A.java:19:33:19:37 | step1 : String | A.java:15:9:15:9 | x : String | provenance |  |
+| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String | provenance | Config |
+| A.java:19:33:19:37 | step1 : String | A.java:19:20:19:38 | apply(...) : String | provenance | Config |
+| A.java:26:8:26:8 | x : String | A.java:26:50:26:50 | x : String | provenance |  |
+| A.java:26:8:26:8 | x : String | A.java:26:75:26:75 | x : String | provenance |  |
+| A.java:26:35:26:56 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String | provenance |  |
+| A.java:26:50:26:50 | x : String | A.java:26:35:26:56 | propagateState(...) : String | provenance | Config |
+| A.java:26:60:26:81 | propagateState(...) : String | A.java:26:13:26:81 | ...?...:... : String | provenance |  |
+| A.java:26:75:26:75 | x : String | A.java:26:60:26:81 | propagateState(...) : String | provenance | Config |
+| A.java:28:20:28:27 | source(...) : String | A.java:29:33:29:37 | step0 : String | provenance |  |
+| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String | provenance |  |
+| A.java:29:20:29:38 | apply(...) : String | A.java:30:33:30:37 | step1 : String | provenance |  |
+| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | provenance |  |
+| A.java:29:33:29:37 | step0 : String | A.java:26:8:26:8 | x : String | provenance |  |
+| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String | provenance | Config |
+| A.java:29:33:29:37 | step0 : String | A.java:29:20:29:38 | apply(...) : String | provenance | Config |
+| A.java:30:20:30:38 | apply(...) : String | A.java:32:10:32:14 | step2 | provenance |  |
+| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | provenance |  |
+| A.java:30:33:30:37 | step1 : String | A.java:26:8:26:8 | x : String | provenance |  |
+| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String | provenance | Config |
+| A.java:30:33:30:37 | step1 : String | A.java:30:20:30:38 | apply(...) : String | provenance | Config |
+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: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 |
+| 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
+#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 |

java/ql/test/library-tests/dataflow/deduplicate-path-graph/test.ql

@@ -0,0 +1,84 @@
+/**
+ * @kind path-problem
+ */
+
+import java
+import semmle.code.java.dataflow.DataFlow
+import DataFlow
+
+MethodCall 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().(MethodCall).getMethod().getName() = "source" and state = ["A", "B"]
+  }
+
+  predicate isSink(Node n, FlowState state) {
+    n.asExpr() = any(MethodCall acc | acc.getMethod().getName() = "sink").getAnArgument() and
+    state = ["A", "B"]
+  }
+
+  predicate isAdditionalFlowStep(Node node1, FlowState state1, Node node2, FlowState state2) {
+    exists(MethodCall 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, _, _) and
+    not Graph::subpaths(prev, node, _, _) // argument-passing edges are handled separately
+    or
+    Graph::subpaths(prev, _, _, node) // arg -> out (should be included in 'edges' but handle the case here for clarity)
+  )
+  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"

ruby/ql/src/queries/security/cwe-094/CodeInjection.ql

@@ -16,20 +16,9 @@
 
 private import codeql.ruby.AST
 private import codeql.ruby.security.CodeInjectionQuery
-import CodeInjectionFlow::PathGraph
+import DataFlow::DeduplicatePathGraph<CodeInjectionFlow::PathNode, CodeInjectionFlow::PathGraph>
 
-from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Source sourceNode
-where
-  CodeInjectionFlow::flowPath(source, sink) and
-  sourceNode = source.getNode() and
-  // removing duplications of the same path, but different flow-labels.
-  sink =
-    min(CodeInjectionFlow::PathNode otherSink |
-      CodeInjectionFlow::flowPath(any(CodeInjectionFlow::PathNode s | s.getNode() = sourceNode),
-        otherSink) and
-      otherSink.getNode() = sink.getNode()
-    |
-      otherSink order by otherSink.getState().getStringRepresentation()
-    )
-select sink.getNode(), source, sink, "This code execution depends on a $@.", sourceNode,
+from PathNode source, PathNode sink
+where CodeInjectionFlow::flowPath(source.getAnOriginalPathNode(), sink.getAnOriginalPathNode())
+select sink.getNode(), source, sink, "This code execution depends on a $@.", source.getNode(),
   "user-provided value"