Merge pull request #5223 from smowton/smowton/feature/backward-dataflow-for-modelled-fluent-methods

Java: Add backward dataflow edges through modelled function invocations

java/ql/src/semmle/code/java/dataflow/FlowSteps.qll

@@ -20,6 +20,57 @@ private module Frameworks {
   private import semmle.code.java.frameworks.ApacheHttp
 }
 
+/**
+ * A method that returns the exact value of one of its parameters or the qualifier.
+ *
+ * Extend this class and override `returnsValue` to add additional value-preserving steps through a
+ * method that should be added to the basic local flow step relation.
+ *
+ * These steps will be visible for all global data-flow purposes, as well as via
+ * `DataFlow::Node.getASuccessor` and other related functions exposing intraprocedural dataflow.
+ */
+abstract class ValuePreservingMethod extends Method {
+  /**
+   * Holds if this method returns precisely the value passed into argument `arg`.
+   * `arg` is a parameter index, or is -1 to indicate the qualifier.
+   */
+  abstract predicate returnsValue(int arg);
+}
+
+/**
+ * A method that returns the exact value of its qualifier (e.g., `return this;`)
+ *
+ * Extend this class to add additional value-preserving steps from qualifier to return value through a
+ * method that should be added to the basic local flow step relation.
+ *
+ * These steps will be visible for all global data-flow purposes, as well as via
+ * `DataFlow::Node.getASuccessor` and other related functions exposing intraprocedural dataflow.
+ */
+abstract class FluentMethod extends ValuePreservingMethod {
+  override predicate returnsValue(int arg) { arg = -1 }
+}
+
+private class StandardLibraryValuePreservingMethod extends ValuePreservingMethod {
+  int returnsArgNo;
+
+  StandardLibraryValuePreservingMethod() {
+    this.getDeclaringType().hasQualifiedName("java.util", "Objects") and
+    (
+      this.hasName(["requireNonNull", "requireNonNullElseGet"]) and returnsArgNo = 0
+      or
+      this.hasName("requireNonNullElse") and returnsArgNo = [0 .. this.getNumberOfParameters() - 1]
+      or
+      this.hasName("toString") and returnsArgNo = 1
+    )
+    or
+    this.getDeclaringType().getASourceSupertype*().hasQualifiedName("java.util", "Stack") and
+    this.hasName("push") and
+    returnsArgNo = 0
+  }
+
+  override predicate returnsValue(int argNo) { argNo = returnsArgNo }
+}
+
 /**
  * A unit class for adding additional taint steps.
  *

java/ql/src/semmle/code/java/dataflow/internal/DataFlowUtil.qll

@@ -8,6 +8,7 @@ private import semmle.code.java.dataflow.SSA
 private import semmle.code.java.dataflow.TypeFlow
 private import semmle.code.java.controlflow.Guards
 private import semmle.code.java.dataflow.ExternalFlow
+private import semmle.code.java.dataflow.FlowSteps
 import semmle.code.java.dataflow.InstanceAccess
 
 cached
@@ -408,28 +409,11 @@ predicate simpleLocalFlowStep(Node node1, Node node2) {
   or
   summaryStep(node1, node2, "value")
   or
-  exists(MethodAccess ma, Method m |
-    ma = node2.asExpr() and
-    m = ma.getMethod() and
-    m.getDeclaringType().hasQualifiedName("java.util", "Objects") and
-    (
-      m.hasName(["requireNonNull", "requireNonNullElseGet"]) and node1.asExpr() = ma.getArgument(0)
-      or
-      m.hasName("requireNonNullElse") and node1.asExpr() = ma.getAnArgument()
-      or
-      m.hasName("toString") and node1.asExpr() = ma.getArgument(1)
-    )
-  )
-  or
-  exists(MethodAccess ma, Method m |
-    ma = node2.asExpr() and
-    m = ma.getMethod() and
-    m.getDeclaringType()
-        .getSourceDeclaration()
-        .getASourceSupertype*()
-        .hasQualifiedName("java.util", "Stack") and
-    m.hasName("push") and
-    node1.asExpr() = ma.getArgument(0)
+  exists(MethodAccess ma, ValuePreservingMethod m, int argNo |
+    ma.getCallee().getSourceDeclaration() = m and m.returnsValue(argNo)
+  |
+    node2.asExpr() = ma and
+    node1.(ArgumentNode).argumentOf(ma, argNo)
   )
 }
 

java/ql/test/library-tests/dataflow/fluent-methods/Test.java

@@ -6,6 +6,16 @@ public class Test {
     return this;
   }
 
+  public Test modelledFluentMethod() {
+    // A model in the accompanying .ql file will indicate that the qualifier flows to the return value.
+    return null;
+  }
+
+  public static Test modelledIdentity(Test t) {
+    // A model in the accompanying .ql file will indicate that the argument flows to the return value.
+    return null;
+  }
+
   public Test indirectlyFluentNoop() {
     return this.fluentNoop();
   }
@@ -47,4 +57,16 @@ public class Test {
     sink(t.get()); // $hasTaintFlow=y
   }
 
+  public static void testModel1() {
+    Test t = new Test();
+    t.indirectlyFluentNoop().modelledFluentMethod().fluentSet(source()).fluentNoop();
+    sink(t.get()); // $hasTaintFlow=y
+  }
+
+  public static void testModel2() {
+    Test t = new Test();
+    Test.modelledIdentity(t).indirectlyFluentNoop().modelledFluentMethod().fluentSet(source()).fluentNoop();
+    sink(t.get()); // $hasTaintFlow=y
+  }
+
 }

java/ql/test/library-tests/dataflow/fluent-methods/flow.ql

@@ -1,5 +1,6 @@
 import java
 import semmle.code.java.dataflow.DataFlow
+import semmle.code.java.dataflow.FlowSteps
 import TestUtilities.InlineExpectationsTest
 
 class Conf extends DataFlow::Configuration {
@@ -14,6 +15,16 @@ class Conf extends DataFlow::Configuration {
   }
 }
 
+class Model extends FluentMethod {
+  Model() { this.getName() = "modelledFluentMethod" }
+}
+
+class IdentityModel extends ValuePreservingMethod {
+  IdentityModel() { this.getName() = "modelledIdentity" }
+
+  override predicate returnsValue(int arg) { arg = 0 }
+}
+
 class HasFlowTest extends InlineExpectationsTest {
   HasFlowTest() { this = "HasFlowTest" }