Merge pull request github#5219 from smowton/smowton/feature/backward-dataflow-for-fluent-methods

Java: Add backward dataflow edges through fluent function invocations.

Author: aschackmull

cpp/change-notes/2021-03-01-fluent-interface-data-flow.md

@@ -0,0 +1,2 @@
+lgtm,codescanning
+* The data-flow library now recognises more side-effects of method chaining (e.g. `someObject.setX(clean).setY(tainted).setZ...` having a side-effect on `someObject`), as well as other related circumstances where a function input is directly passed to its output. All queries that use data-flow analysis, including most security queries, may return more results accordingly.

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl2.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl3.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImpl4.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll

@@ -415,6 +415,30 @@ private module Cached {
     store(node1, tc.getContent(), node2, contentType, tc.getContainerType())
   }
 
+  /**
+   * Holds if data can flow from `fromNode` to `toNode` because they are the post-update
+   * nodes of some function output and input respectively, where the output and input
+   * are aliases. A typical example is a function returning `this`, implementing a fluent
+   * interface.
+   */
+  cached
+  predicate reverseStepThroughInputOutputAlias(PostUpdateNode fromNode, PostUpdateNode toNode) {
+    exists(Node fromPre, Node toPre |
+      fromPre = fromNode.getPreUpdateNode() and
+      toPre = toNode.getPreUpdateNode()
+    |
+      exists(DataFlowCall c |
+        // Does the language-specific simpleLocalFlowStep already model flow
+        // from function input to output?
+        fromPre = getAnOutNode(c, _) and
+        toPre.(ArgumentNode).argumentOf(c, _) and
+        simpleLocalFlowStep(toPre.(ArgumentNode), fromPre)
+      )
+      or
+      argumentValueFlowsThrough(toPre, TReadStepTypesNone(), fromPre)
+    )
+  }
+
   /**
    * Holds if the call context `call` either improves virtual dispatch in
    * `callable` or if it allows us to prune unreachable nodes in `callable`.

cpp/ql/src/semmle/code/cpp/dataflow/internal/DataFlowImplLocal.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl2.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowImpl3.qll

@@ -223,7 +223,10 @@ private predicate isAdditionalFlowStep(
  * Holds if data can flow in one local step from `node1` to `node2`.
  */
 private predicate localFlowStep(Node node1, Node node2, Configuration config) {
-  simpleLocalFlowStep(node1, node2) and
+  (
+    simpleLocalFlowStep(node1, node2) or
+    reverseStepThroughInputOutputAlias(node1, node2)
+  ) and
   not outBarrier(node1, config) and
   not inBarrier(node2, config) and
   not fullBarrier(node1, config) and