move new predicates to a more fitting location

Author: erik-krogh

javascript/ql/src/Security/CWE-352/MissingCsrfMiddleware.ql

@@ -50,7 +50,7 @@ private DataFlow::SourceNode getARouteUsingCookies(DataFlow::TypeTracker t) {
     result = pred.track(t2, t)
     or
     t = t2 and
-    Express::routeHandlerStep(pred, result)
+    HTTP::routeHandlerStep(pred, result)
   )
 }
 

javascript/ql/src/semmle/javascript/frameworks/Express.qll

@@ -72,50 +72,6 @@ module Express {
     result = "del"
   }
 
-  /**
-   * Holds if `call` decorates the function `pred`.
-   * This means that `call` returns a function that forwards its arguments to `pred`.
-   */
-  predicate isDecoratedCall(DataFlow::CallNode call, DataFlow::FunctionNode decoratee) {
-    // indirect route-handler `result` is given to function `outer`, which returns function `inner` which calls the function `pred`.
-    exists(int i, Function outer, Function inner |
-      decoratee = call.getArgument(i).getALocalSource() and
-      outer = call.getACallee() and
-      inner = outer.getAReturnedExpr() and
-      isAForwardingRouteHandlerCall(DataFlow::parameterNode(outer.getParameter(i)), inner.flow())
-    )
-  }
-
-  /**
-   * Holds if `f` looks like a route-handler and a call to `callee` inside `f` forwards all of the parameters from `f` to that call, 
-   */
-  private predicate isAForwardingRouteHandlerCall(DataFlow::SourceNode callee, HTTP::RouteHandlerCandidate f) {
-    exists(DataFlow::CallNode call | call = callee.getACall() |
-      forall(int arg | arg = [0 .. f.getNumParameter() - 1] |
-        f.getParameter(arg).flowsTo(call.getArgument(arg))
-      ) and
-      call.getContainer() = f.getFunction()
-    )
-  }
-
-  /**
-   * Holds if there exists a step from `pred` to `succ` for a RouteHandler - beyond the usual steps defined by TypeTracking.
-   */
-  predicate routeHandlerStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) {
-    isDecoratedCall(succ, pred)
-    or
-    // A forwarding call
-    isAForwardingRouteHandlerCall(pred, succ)
-    or
-    // a container containing route-handlers.
-    exists(HTTP::RouteHandlerCandidateContainer container | pred = container.getRouteHandler(succ))
-    or
-    // (function (req, res) {}).bind(this);
-    exists(DataFlow::PartialInvokeNode call |
-      succ = call.getBoundFunction(pred, 0)
-    )
-  }
-
   /**
    * A call to an Express router method that sets up a route.
    */
@@ -169,7 +125,7 @@ module Express {
       exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ | succ = getARouteHandler(t2) |
         result = succ.backtrack(t2, t)
         or
-        routeHandlerStep(result, succ) and
+        HTTP::routeHandlerStep(result, succ) and
         t = t2
       )
     }

javascript/ql/src/semmle/javascript/frameworks/HTTP.qll

@@ -233,6 +233,51 @@ module HTTP {
     ResponseExpr getAResponseExpr() { result.getRouteHandler() = this }
   }
 
+  /**
+   * Holds if `call` decorates the function `pred`.
+   * This means that `call` returns a function that forwards its arguments to `pred`. 
+   * Only holds when the decorator looks like it is decorating a route-handler.
+   */
+  private predicate isDecoratedCall(DataFlow::CallNode call, DataFlow::FunctionNode decoratee) {
+    // indirect route-handler `result` is given to function `outer`, which returns function `inner` which calls the function `pred`.
+    exists(int i, Function outer, Function inner |
+      decoratee = call.getArgument(i).getALocalSource() and
+      outer = call.getACallee() and
+      inner = outer.getAReturnedExpr() and
+      isAForwardingRouteHandlerCall(DataFlow::parameterNode(outer.getParameter(i)), inner.flow())
+    )
+  }
+
+  /**
+   * Holds if `f` looks like a route-handler and a call to `callee` inside `f` forwards all of the parameters from `f` to that call,
+   */
+  private predicate isAForwardingRouteHandlerCall(
+    DataFlow::SourceNode callee, HTTP::RouteHandlerCandidate f
+  ) {
+    exists(DataFlow::CallNode call | call = callee.getACall() |
+      forall(int arg | arg = [0 .. f.getNumParameter() - 1] |
+        f.getParameter(arg).flowsTo(call.getArgument(arg))
+      ) and
+      call.getContainer() = f.getFunction()
+    )
+  }
+
+  /**
+   * Holds if there exists a step from `pred` to `succ` for a RouteHandler - beyond the usual steps defined by TypeTracking.
+   */
+  predicate routeHandlerStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) {
+    isDecoratedCall(succ, pred)
+    or
+    // A forwarding call
+    isAForwardingRouteHandlerCall(pred, succ)
+    or
+    // a container containing route-handlers.
+    exists(HTTP::RouteHandlerCandidateContainer container | pred = container.getRouteHandler(succ))
+    or
+    // (function (req, res) {}).bind(this);
+    exists(DataFlow::PartialInvokeNode call | succ = call.getBoundFunction(pred, 0))
+  }
+
   /**
    * An expression that sets up a route on a server.
    */
@@ -596,7 +641,7 @@ module HTTP {
     DataFlow::SourceNode getAPossiblyDecoratedHandler(RouteHandlerCandidate candidate) {
       result = candidate
       or
-      Express::isDecoratedCall(result, candidate)
+      isDecoratedCall(result, candidate)
     }
 
     /**

javascript/ql/src/semmle/javascript/frameworks/NodeJSLib.qll

@@ -232,7 +232,7 @@ module NodeJSLib {
         result = succ.backtrack(t2, t)
         or
         t = t2 and
-        Express::routeHandlerStep(result, succ)
+        HTTP::routeHandlerStep(result, succ)
       )
     }