Ratpack: Drop support for flatMap like methods

Author: JLLeitschuh

java/ql/src/semmle/code/java/frameworks/ratpack/RatpackExec.qll

@@ -54,7 +54,7 @@ abstract private class SimpleFluentLambdaMethod extends FluentLambdaMethod {
 private class RatpackPromiseMapMethod extends SimpleFluentLambdaMethod {
   RatpackPromiseMapMethod() {
     getDeclaringType() instanceof RatpackPromise and
-    hasName(["map", "flatMap", "blockingMap", "apply"])
+    hasName(["map", "blockingMap"]) // "flatMap" & "apply" cause false positives. Wait for fluent lambda support.
   }
 
   override predicate consumesTaint(int lambdaArg) { lambdaArg = 0 }
@@ -63,15 +63,14 @@ private class RatpackPromiseMapMethod extends SimpleFluentLambdaMethod {
 }
 
 /**
- * Represents the `mapIf` and `flatMapIf` method.
+ * Represents the `mapIf` method.
  *
  * `<O> Promise<O> mapIf(Predicate<T> predicate, Function<T, O> onTrue, Function<T, O> onFalse)`
- * `<O> Promise<O> flatMapIf(Predicate<T> predicate, Function<T, Promise<O>> onTrue, Function<T, Promise<O>> onFalse)`
  */
 private class RatpackPromiseMapIfMethod extends FluentLambdaMethod {
   RatpackPromiseMapIfMethod() {
     getDeclaringType() instanceof RatpackPromise and
-    hasName(["mapIf", "flatMapIf"]) and
+    hasName(["mapIf"]) and // "flatMapIf" causes false positives. Wait for fluent lambda support.
     getNumberOfParameters() = 3
   }
 
@@ -85,7 +84,7 @@ private class RatpackPromiseMapIfMethod extends FluentLambdaMethod {
 private class RatpackPromiseMapErrorMethod extends FluentLambdaMethod {
   RatpackPromiseMapErrorMethod() {
     getDeclaringType() instanceof RatpackPromise and
-    hasName(["mapError", "flatMapError"])
+    hasName(["mapError"]) // "flatMapError" causes false positives. Wait for fluent lambda support.
   }
 
   override predicate consumesTaint(int methodArg, int lambdaArg) { none() }
@@ -121,41 +120,41 @@ private class RatpackPromiseFluentMethod extends FluentMethod, FluentLambdaMetho
     hasName(["route"]) and methodArg = [0, 1] and lambdaArg = 0
   }
 
-  override predicate doesReturnTaint(int arg) { hasName(["flatMapIf"]) and arg = 1 }
+  override predicate doesReturnTaint(int arg) { none() } // "flatMapIf" causes false positives. Wait for fluent lambda support.
+}
+
+/**
+ * Holds if the method access qualifier `node1` has dataflow to the functional expression parameter `node2`.
+ */
+private predicate stepIntoLambda(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(MethodAccess ma, FluentLambdaMethod flm, int methodArg, int lambdaArg |
+    flm.consumesTaint(methodArg, lambdaArg)
+  |
+    ma.getMethod() = flm and
+    node1.asExpr() = ma.getQualifier() and
+    ma.getArgument(methodArg).(FunctionalExpr).asMethod().getParameter(lambdaArg) =
+      node2.asParameter()
+  )
+}
+
+/**
+ * Holds if the return statement result of the functional expression `node1` has dataflow to the
+ * method access result `node2`.
+ */
+private predicate stepOutOfLambda(DataFlow::Node node1, DataFlow::Node node2) {
+  exists(FluentLambdaMethod flm, MethodAccess ma, FunctionalExpr fe, int arg |
+    flm.doesReturnTaint(arg)
+  |
+    fe.asMethod().getBody().getAStmt().(ReturnStmt).getResult() = node1.asExpr() and
+    ma.getMethod() = flm and
+    node2.asExpr() = ma and
+    ma.getArgument(arg) = fe
+  )
 }
 
 private class RatpackPromiseTaintPreservingStep extends AdditionalTaintStep {
   override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
     stepIntoLambda(node1, node2) or
     stepOutOfLambda(node1, node2)
   }
-
-  /**
-   * Holds if the method access qualifier `node1` has dataflow to the functional expression parameter `node2`.
-   */
-  predicate stepIntoLambda(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(MethodAccess ma, FluentLambdaMethod flm, int methodArg, int lambdaArg |
-      flm.consumesTaint(methodArg, lambdaArg)
-    |
-      ma.getMethod() = flm and
-      node1.asExpr() = ma.getQualifier() and
-      ma.getArgument(methodArg).(FunctionalExpr).asMethod().getParameter(lambdaArg) =
-        node2.asParameter()
-    )
-  }
-
-  /**
-   * Holds if the return statement result of the functional expression `node1` has dataflow to the
-   * method access result `node2`.
-   */
-  predicate stepOutOfLambda(DataFlow::Node node1, DataFlow::Node node2) {
-    exists(FluentLambdaMethod flm, MethodAccess ma, FunctionalExpr fe, int arg |
-      flm.doesReturnTaint(arg)
-    |
-      fe.asMethod().getBody().getAStmt().(ReturnStmt).getResult() = node1.asExpr() and
-      ma.getMethod() = flm and
-      node2.asExpr() = ma and
-      ma.getArgument(arg) = fe
-    )
-  }
 }

java/ql/test/library-tests/frameworks/ratpack/resources/Resource.java

@@ -5,6 +5,7 @@
 import ratpack.core.parse.Parse;
 import ratpack.exec.Promise;
 import ratpack.func.Action;
+import ratpack.func.Function;
 import java.io.OutputStream;
 
 class Resource {
@@ -67,7 +68,7 @@ void test4() {
         sink(Promise.value(tainted)); //$hasTaintFlow
         Promise
             .value(tainted)
-            .flatMap(a -> Promise.value(a))
+            .map(a -> a)
             .then(this::sink); //$hasTaintFlow
     }
 
@@ -180,33 +181,51 @@ void test8() {
             .then(value -> {
                 sink(value); //$hasTaintFlow
             });
+        // Waiting for support for lambda data flow
+        // Promise
+        //     .value("potato")
+        //     .flatMapError(RuntimeException.class, exception -> {
+        //         return Promise.value(taint());
+        //     })
+        //     .then(value -> {
+        //         sink(value); //$hasTaintFlow
+        //     });
+    }
+
+    void test9() {
+        String tainted = taint();
         Promise
-            .value("potato")
-            .flatMapError(RuntimeException.class, exception -> {
-                return Promise.value(taint());
-            })
+            .value(tainted)
+            .map(Resource::identity)
             .then(value -> {
                 sink(value); //$hasTaintFlow
             });
+        Promise
+            .value("potato")
+            .map(Resource::identity)
+            .then(value -> {
+                sink(value); // no taints flow
+            });
     }
 
-    void test9() {
+    public static String identity(String input) {
+        return input;
+    }
+
+    void test10() {
         String tainted = taint();
         Promise
             .value(tainted)
-            .apply(Resource::identity)
+            .map(a -> a)
             .then(value -> {
                 sink(value); //$hasTaintFlow
             });
         Promise
             .value("potato")
-            .apply(Resource::identity)
+            .map(a -> a)
             .then(value -> {
                 sink(value); // no taints flow
             });
     }
-
-    public static Promise<String> identity(Promise<String> input) {
-        return input.map(i -> i);
-    }
+    
 }