add additional indirect route-handler steps

Author: erik-krogh

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

@@ -73,24 +73,51 @@ module Express {
   }
 
   /**
-   * Holds if there exists a step from `pred` to `succ` for a RouteHandler - beyond the usual steps defined by TypeTracking.
+   * Holds if `call` decorates the function `pred`.
+   * This means that `call` returns a function that forwards its arguments to `pred`.
    */
-  predicate routeHandlerStep(DataFlow::SourceNode pred, DataFlow::SourceNode succ) {
+  predicate decoratedRouteHandler(DataFlow::SourceNode pred, DataFlow::CallNode call) {
     // indirect route-handler `result` is given to function `outer`, which returns function `inner` which calls the function `pred`.
-    exists(int i, DataFlow::CallNode call, Function outer, Function inner | call = succ |
+    exists(int i, Function outer, Function inner |
       pred = call.getArgument(i).getALocalSource() and
       outer = call.getACallee() and
       inner = outer.getAReturnedExpr() and
-      exists(DataFlow::CallNode innerCall |
-        innerCall = DataFlow::parameterNode(outer.getParameter(i)).getACall() and
-        forall(int arg | arg = [0, 1] |
-          DataFlow::parameterNode(inner.getParameter(arg)).flowsTo(innerCall.getArgument(arg))
-        )
-      )
+      forwardingCall(DataFlow::parameterNode(outer.getParameter(i)), inner.flow())
+    )
+  }
+
+  /**
+   * Holds if a call to `callee` inside `f` forwards all of the parameters from `f` to that call.
+   */
+  private predicate forwardingCall(DataFlow::SourceNode callee, DataFlow::FunctionNode f) {
+    exists(DataFlow::CallNode call | call = callee.getACall() |
+      f.getNumParameter() >= 2 and
+      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) {
+    decoratedRouteHandler(pred, succ)
+    or
+    // A forwarding call
+    forwardingCall(pred, succ)
     or
     // a container containing route-handlers.
     exists(HTTP::RouteHandlerCandidateContainer container | pred = container.getRouteHandler(succ))
+    or
+    // (function (req, res) {}).bind(this);
+    exists(DataFlow::MethodCallNode call |
+      call.getMethodName() = "bind" and call.getNumArgument() = 1
+    |
+      succ = call and
+      pred = call.getReceiver().getALocalSource()
+    )
   }
 
   /**

javascript/ql/test/library-tests/frameworks/NodeJSLib/tests.expected

@@ -18,6 +18,7 @@ test_RequestInputAccess
 | src/https.js:6:26:6:32 | req.url | url | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:8:3:8:20 | req.headers.cookie | cookie | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:9:3:9:17 | req.headers.foo | header | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
+| src/indirect.js:17:28:17:34 | req.url | url | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
 test_RouteHandler_getAResponseHeader
 | src/http.js:4:32:10:1 | functio ... .foo;\\n} | location | src/http.js:7:3:7:42 | res.wri ... rget }) |
 | src/http.js:12:19:16:1 | functio ... ar");\\n} | content-type | src/http.js:13:3:13:44 | res.set ... /html') |
@@ -55,6 +56,8 @@ test_ResponseExpr
 | src/https.js:13:3:13:5 | res | src/https.js:12:20:16:1 | functio ... ar");\\n} |
 | src/https.js:14:3:14:5 | res | src/https.js:12:20:16:1 | functio ... ar");\\n} |
 | src/https.js:15:3:15:5 | res | src/https.js:12:20:16:1 | functio ... ar");\\n} |
+| src/indirect.js:16:26:16:28 | res | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
+| src/indirect.js:19:38:19:40 | res | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
 test_HeaderDefinition
 | src/http.js:7:3:7:42 | res.wri ... rget }) | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
 | src/http.js:13:3:13:44 | res.set ... /html') | src/http.js:12:19:16:1 | functio ... ar");\\n} |
@@ -128,6 +131,8 @@ test_RouteHandler_getAResponseExpr
 | src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:13:3:13:5 | res |
 | src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:14:3:14:5 | res |
 | src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:15:3:15:5 | res |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:16:26:16:28 | res |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:19:38:19:40 | res |
 test_ServerDefinition_getARouteHandler
 | createServer.js:2:1:2:42 | https.c ... es) {}) | createServer.js:2:20:2:41 | functio ... res) {} |
 | createServer.js:3:1:3:45 | https.c ... es) {}) | createServer.js:3:23:3:44 | functio ... res) {} |
@@ -140,6 +145,7 @@ test_ServerDefinition_getARouteHandler
 | src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:68:12:68:27 | (req,res) => f() |
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:12:1:16:2 | https.c ... r");\\n}) | src/https.js:12:20:16:1 | functio ... ar");\\n} |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
 test_ResponseSendArgument
 | src/http.js:14:13:14:17 | "foo" | src/http.js:12:19:16:1 | functio ... ar");\\n} |
 | src/http.js:15:11:15:15 | "bar" | src/http.js:12:19:16:1 | functio ... ar");\\n} |
@@ -163,7 +169,10 @@ test_RouteSetup_getARouteHandler
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:12:1:16:2 | https.c ... r");\\n}) | src/https.js:12:20:16:1 | functio ... ar");\\n} |
 | src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:14:19:21:3 | return of method requestHandler |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
 | src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:16:12:20:16 | functio ... d(this) |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:17:21:17:35 | routes[req.url] |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:17:40:17:50 | routes['*'] |
 | src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:34:32:34:57 | appServ ... ndler() |
 test_ClientRequest_getADataNode
 | src/http.js:27:16:27:73 | http.re ... POST'}) | src/http.js:50:16:50:22 | 'stuff' |
@@ -181,6 +190,7 @@ test_RemoteFlowSources
 | src/https.js:6:26:6:32 | req.url |
 | src/https.js:8:3:8:20 | req.headers.cookie |
 | src/https.js:9:3:9:17 | req.headers.foo |
+| src/indirect.js:17:28:17:34 | req.url |
 test_RouteHandler
 | createServer.js:2:20:2:41 | functio ... res) {} | createServer.js:2:1:2:42 | https.c ... es) {}) |
 | createServer.js:3:23:3:44 | functio ... res) {} | createServer.js:3:1:3:45 | https.c ... es) {}) |
@@ -193,6 +203,7 @@ test_RouteHandler
 | src/http.js:68:12:68:27 | (req,res) => f() | src/http.js:70:1:70:36 | http.cr ... dler()) |
 | src/https.js:4:33:10:1 | functio ... .foo;\\n} | src/https.js:4:14:10:2 | https.c ... foo;\\n}) |
 | src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:12:1:16:2 | https.c ... r");\\n}) |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:34:14:34:58 | http.cr ... dler()) |
 test_RequestExpr
 | createServer.js:2:30:2:32 | req | createServer.js:2:20:2:41 | functio ... res) {} |
 | createServer.js:3:33:3:35 | req | createServer.js:3:23:3:44 | functio ... res) {} |
@@ -212,6 +223,9 @@ test_RequestExpr
 | src/https.js:8:3:8:5 | req | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:9:3:9:5 | req | src/https.js:4:33:10:1 | functio ... .foo;\\n} |
 | src/https.js:12:29:12:31 | req | src/https.js:12:20:16:1 | functio ... ar");\\n} |
+| src/indirect.js:16:21:16:23 | req | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
+| src/indirect.js:17:28:17:30 | req | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
+| src/indirect.js:19:33:19:35 | req | src/indirect.js:16:12:20:5 | functio ... ;\\n    } |
 test_SystemCommandExecution_getAnArgumentForCommand
 | exec.js:3:1:3:38 | cp.exec ... "], cb) | exec.js:3:21:3:33 | ["--version"] |
 | exec.js:4:1:4:47 | cp.exec ... sion"]) | exec.js:4:23:4:46 | ["-c",  ... rsion"] |
@@ -240,3 +254,6 @@ test_RouteHandler_getARequestExpr
 | src/https.js:4:33:10:1 | functio ... .foo;\\n} | src/https.js:8:3:8:5 | req |
 | src/https.js:4:33:10:1 | functio ... .foo;\\n} | src/https.js:9:3:9:5 | req |
 | src/https.js:12:20:16:1 | functio ... ar");\\n} | src/https.js:12:29:12:31 | req |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:16:21:16:23 | req |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:17:28:17:30 | req |
+| src/indirect.js:16:12:20:5 | functio ... ;\\n    } | src/indirect.js:19:33:19:35 | req |