support indirect route-handlers for NodeJS

Author: erik-krogh

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

@@ -228,7 +228,12 @@ module NodeJSLib {
       t.start() and
       result = handler.flow().getALocalSource()
       or
-      exists(DataFlow::TypeBackTracker t2 | result = getARouteHandler(t2).backtrack(t2, t))
+      exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ | succ = getARouteHandler(t2) |
+        result = succ.backtrack(t2, t)
+        or
+        t = t2 and
+        Express::routeHandlerStep(result, succ)
+      )
     }
 
     override Expr getServer() { result = server }
@@ -727,9 +732,6 @@ module NodeJSLib {
           or
           // heuristic: does not return anything (Node.js will not use the return value)
           exists(astNode.getAReturnStmt().getExpr())
-          or
-          // heuristic: is not invoked (Node.js invokes this at a call site we cannot reason precisely about)
-          exists(DataFlow::InvokeNode cs | cs.getACallee() = astNode)
         )
       )
     }

javascript/ql/test/library-tests/frameworks/NodeJSLib/src/indirect.js

@@ -0,0 +1,36 @@
+var http = require('http');
+
+function decorate(method) {
+  return function(req, res) {
+    return method.call(this, req, res);
+  };
+}
+
+function Server(routes) {
+  this.routes = routes;
+}
+
+Server.prototype = {
+  requestHandler: function() {
+    var routes = this.routes;
+    return function(req, res) { // route handler
+      var handler = routes[req.url] || routes['*'];
+
+      return handler.call(this, req, res);
+    }.bind(this);
+  },
+};
+
+var routes = {
+  '/foo/bar': decorate((req, res) => { // route handler
+    res.end("foo");
+  }),
+  '/bar/foo': function(req, res) { // route handler
+    res.end("bar");
+  }
+};
+
+var appServer = new Server(routes);
+var server = http.createServer(appServer.requestHandler());
+
+server.listen(8080, () => {});

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

@@ -10,6 +10,7 @@ test_isCreateServer
 | src/http.js:70:1:70:36 | http.cr ... dler()) |
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) |
 | src/https.js:12:1:16:2 | https.c ... r");\\n}) |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) |
 test_RequestInputAccess
 | src/http.js:6:26:6:32 | req.url | url | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
 | src/http.js:8:3:8:20 | req.headers.cookie | cookie | src/http.js:4:32:10:1 | functio ... .foo;\\n} |
@@ -72,6 +73,7 @@ test_RouteSetup_getServer
 | src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:70:1:70:36 | http.cr ... dler()) |
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) | src/https.js:4:14:10:2 | https.c ... foo;\\n}) |
 | src/https.js:12:1:16:2 | https.c ... r");\\n}) | src/https.js:12:1:16:2 | https.c ... r");\\n}) |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) | src/indirect.js:34:14:34:58 | http.cr ... dler()) |
 test_ClientRequest
 | src/http.js:18:1:18:30 | http.re ... uth" }) |
 | src/http.js:21:15:26:6 | http.re ... \\n    }) |
@@ -94,6 +96,7 @@ test_ServerDefinition
 | src/http.js:70:1:70:36 | http.cr ... dler()) |
 | src/https.js:4:14:10:2 | https.c ... foo;\\n}) |
 | src/https.js:12:1:16:2 | https.c ... r");\\n}) |
+| src/indirect.js:34:14:34:58 | http.cr ... dler()) |
 test_HeaderAccess
 | src/http.js:9:3:9:17 | req.headers.foo | foo |
 | src/https.js:9:3:9:17 | req.headers.foo | foo |
@@ -159,6 +162,9 @@ test_RouteSetup_getARouteHandler
 | src/http.js:70:1:70:36 | http.cr ... dler()) | src/http.js:70:19:70:35 | getArrowHandler() |
 | 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:16 | functio ... d(this) |
+| 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' |
 | src/http.js:27:16:27:73 | http.re ... POST'}) | src/http.js:51:14:51:25 | 'more stuff' |