Merge pull request #6841 from hvitved/dataflow/incorrect-summary-chaining

Data flow: Add tests for missing summary flow

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowPrivate.qll

@@ -234,6 +234,19 @@ class DataFlowCall extends TDataFlowCall {
 
   /** Gets the location of this call. */
   abstract Location getLocation();
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  final predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    this.getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
 }
 
 /** A source call, that is, a `Call`. */

java/ql/lib/semmle/code/java/dataflow/internal/FlowSummaryImpl.qll

@@ -870,4 +870,95 @@ module Private {
       )
     }
   }
+
+  /**
+   * Provides query predicates for rendering the generated data flow graph for
+   * a summarized callable.
+   *
+   * Import this module into a `.ql` file of `@kind graph` to render the graph.
+   * The graph is restricted to callables from `RelevantSummarizedCallable`.
+   */
+  module RenderSummarizedCallable {
+    /** A summarized callable to include in the graph. */
+    abstract class RelevantSummarizedCallable extends SummarizedCallable { }
+
+    private newtype TNodeOrCall =
+      MkNode(Node n) {
+        exists(RelevantSummarizedCallable c |
+          n = summaryNode(c, _)
+          or
+          n.(ParamNode).isParameterOf(c, _)
+        )
+      } or
+      MkCall(DataFlowCall call) {
+        call = summaryDataFlowCall(_) and
+        call.getEnclosingCallable() instanceof RelevantSummarizedCallable
+      }
+
+    private class NodeOrCall extends TNodeOrCall {
+      Node asNode() { this = MkNode(result) }
+
+      DataFlowCall asCall() { this = MkCall(result) }
+
+      string toString() {
+        result = this.asNode().toString()
+        or
+        result = this.asCall().toString()
+      }
+
+      /**
+       * Holds if this element is at the specified location.
+       * The location spans column `startcolumn` of line `startline` to
+       * column `endcolumn` of line `endline` in file `filepath`.
+       * For more information, see
+       * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+       */
+      predicate hasLocationInfo(
+        string filepath, int startline, int startcolumn, int endline, int endcolumn
+      ) {
+        this.asNode().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+        or
+        this.asCall().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+      }
+    }
+
+    query predicate nodes(NodeOrCall n, string key, string val) {
+      key = "semmle.label" and val = n.toString()
+    }
+
+    private predicate edgesComponent(NodeOrCall a, NodeOrCall b, string value) {
+      exists(boolean preservesValue |
+        Private::Steps::summaryLocalStep(a.asNode(), b.asNode(), preservesValue) and
+        if preservesValue = true then value = "value" else value = "taint"
+      )
+      or
+      exists(Content c |
+        Private::Steps::summaryReadStep(a.asNode(), c, b.asNode()) and
+        value = "read (" + c + ")"
+        or
+        Private::Steps::summaryStoreStep(a.asNode(), c, b.asNode()) and
+        value = "store (" + c + ")"
+        or
+        Private::Steps::summaryClearsContent(a.asNode(), c) and
+        b = a and
+        value = "clear (" + c + ")"
+      )
+      or
+      summaryPostUpdateNode(b.asNode(), a.asNode()) and
+      value = "post-update"
+      or
+      b.asCall() = summaryDataFlowCall(a.asNode()) and
+      value = "receiver"
+      or
+      exists(int i |
+        summaryArgumentNode(b.asCall(), a.asNode(), i) and
+        value = "argument (" + i + ")"
+      )
+    }
+
+    query predicate edges(NodeOrCall a, NodeOrCall b, string key, string value) {
+      key = "semmle.label" and
+      value = strictconcat(string s | edgesComponent(a, b, s) | s, " / ")
+    }
+  }
 }

java/ql/test/library-tests/dataflow/callback-dispatch/A.java

@@ -169,4 +169,25 @@ public class A {
     sink(res2); // $ flow=19
   }
 
+  static void applyConsumer1Field1Field2(A a1, A a2, Consumer1 con) {
+    // summary:
+    // con.eat(a1.field1);
+    // con.eat(a2.field2);
+  }
+
+  static void wrapSinkToAvoidFieldSsa(A a) { sink(a.field1); }
+
+  void foo3() {
+    A a1 = new A();
+    a1.field1 = source(20);
+    A a2 = new A();
+    applyConsumer1Field1Field2(a1, a2, p -> {
+      sink(p); // MISSING FLOW
+    });
+    wrapSinkToAvoidFieldSsa(a1); // MISSING FLOW
+    sink(a2.field2);
+  }
+
+  public Object field1;
+  public Object field2;
 }

java/ql/test/library-tests/dataflow/callback-dispatch/test.ql

@@ -8,6 +8,8 @@ class SummaryModelTest extends SummaryModelCsv {
     row =
       [
         "my.callback.qltest;A;false;applyConsumer1;(Object,Consumer1);;Argument[0];Parameter[0] of Argument[1];value",
+        "my.callback.qltest;A;false;applyConsumer1Field1Field2;(A,A,Consumer1);;Field[my.callback.qltest.A.field1] of Argument[0];Parameter[0] of Argument[2];value",
+        "my.callback.qltest;A;false;applyConsumer1Field1Field2;(A,A,Consumer1);;Field[my.callback.qltest.A.field2] of Argument[1];Parameter[0] of Argument[2];value",
         "my.callback.qltest;A;false;applyConsumer2;(Object,Consumer2);;Argument[0];Parameter[0] of Argument[1];value",
         "my.callback.qltest;A;false;applyConsumer3;(Object,Consumer3);;Argument[0];Parameter[0] of Argument[1];value",
         "my.callback.qltest;A;false;applyConsumer3_ret_postup;(Consumer3);;Parameter[0] of Argument[0];ReturnValue;value",