Merge pull request #1198 from esben-semmle/js/more-express-route-handlers

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.21/analysis-javascript.md

@@ -7,6 +7,7 @@
   - [socket.io](http://socket.io)
   - [Node.js](http://nodejs.org)
   - [Firebase](https://firebase.google.com/)
+  - [Express](https://expressjs.com/)
 
 * The security queries now track data flow through Base64 decoders such as the Node.js `Buffer` class, the DOM function `atob`, and a number of npm packages intcluding [`abab`](https://www.npmjs.com/package/abab), [`atob`](https://www.npmjs.com/package/atob), [`btoa`](https://www.npmjs.com/package/btoa), [`base-64`](https://www.npmjs.com/package/base-64), [`js-base64`](https://www.npmjs.com/package/js-base64), [`Base64.js`](https://www.npmjs.com/package/Base64) and [`base64-js`](https://www.npmjs.com/package/base64-js).
 

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

@@ -14,10 +14,8 @@ module Express {
    * Express application.
    */
   DataFlow::SourceNode appCreation() {
-    exists(DataFlow::ModuleImportNode express | express.getPath() = "express" |
-      // `app = [new] express()`
-      result = express.getAnInvocation()
-    )
+    // `app = [new] express()`
+    result = DataFlow::moduleImport("express").getAnInvocation()
     or
     // `app = express.createServer()`
     result = DataFlow::moduleMember("express", "createServer").getAnInvocation()
@@ -40,7 +38,13 @@ module Express {
   private predicate isRouter(Expr e, RouterDefinition router) {
     router.flowsTo(e)
     or
-    isRouter(e.(RouteSetup).getReceiver(), router)
+    exists (DataFlow::MethodCallNode chain, DataFlow::Node base, string name |
+      name = "route" or
+      name = routeSetupMethodName() |
+      chain.calls(base, name) and
+      isRouter(base.asExpr(), router) and
+      chain.flowsToExpr(e)
+    )
   }
 
   /**
@@ -50,31 +54,35 @@ module Express {
     RouterDefinition router;
 
     RouteExpr() {
-      isRouter(this.getReceiver(), router) and
-      this.getMethodName() = "route"
-      or
-      this.(RouteSetup).getReceiver().(RouteExpr).getRouter() = router
+      isRouter(this, router)
     }
 
     /** Gets the router from which this route was created. */
     RouterDefinition getRouter() { result = router }
   }
 
   /**
-   * A call to an Express method that sets up a route.
+   * Gets the name of an Express router method that sets up a route.
+   */
+  string routeSetupMethodName() {
+    result = "param" or
+    result = "all" or
+    result = "use" or
+    result = any(HTTP::RequestMethodName m).toLowerCase() or
+    // deprecated methods
+    result = "error" or
+    result = "del"
+  }
+
+  /**
+   * A call to an Express router method that sets up a route.
    */
   class RouteSetup extends HTTP::Servers::StandardRouteSetup, MethodCallExpr {
     RouterDefinition router;
 
     RouteSetup() {
-      exists(string methodName | methodName = getMethodName() |
-        (isRouter(getReceiver(), router) or getReceiver().(RouteExpr).getRouter() = router) and
-        (
-          methodName = "all" or
-          methodName = "use" or
-          methodName = any(HTTP::RequestMethodName m).toLowerCase()
-        )
-      )
+      isRouter(getReceiver(), router) and
+      getMethodName() = routeSetupMethodName() 
     }
 
     /** Gets the path associated with the route. */

javascript/ql/test/library-tests/frameworks/Express/src/routesetups.js

@@ -0,0 +1,12 @@
+var express = require('express');
+
+express.Router()
+	.param('', h)
+	.get('', h);
+
+var app = express.createServer();
+app.error(h);
+
+var router = express.Router();
+var root = router.route('/');
+root.post('', h);

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

@@ -66,6 +66,10 @@ test_RouteSetup_getLastRouteHandlerExpr
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | src/responseExprs.js:13:23:15:1 | functio ... res4;\\n} |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | src/responseExprs.js:16:30:42:1 | functio ...     }\\n} |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | src/route.js:5:12:5:38 | functio ... ext) {} |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | src/routesetups.js:4:13:4:13 | h |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | src/routesetups.js:5:11:5:11 | h |
+| src/routesetups.js:8:1:8:12 | app.error(h) | src/routesetups.js:8:11:8:11 | h |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:12:15:12:15 | h |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:4:19:4:25 | protect |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:5:14:5:28 | makeSubRouter() |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | src/subrouter.js:9:27:9:34 | handler1 |
@@ -219,6 +223,10 @@ test_RouteSetup_getRouter
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | src/responseExprs.js:2:11:2:19 | express() |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | src/responseExprs.js:2:11:2:19 | express() |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | src/route.js:2:14:2:29 | express.Router() |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:8:1:8:12 | app.error(h) | src/routesetups.js:7:11:7:32 | express ... erver() |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:10:14:10:29 | express.Router() |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:2:11:2:19 | express() |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:2:11:2:19 | express() |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | src/subrouter.js:8:16:8:31 | express.Router() |
@@ -483,6 +491,10 @@ test_RouteHandlerExpr
 | src/responseExprs.js:13:23:15:1 | functio ... res4;\\n} | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | true |
 | src/responseExprs.js:16:30:42:1 | functio ...     }\\n} | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | true |
 | src/route.js:5:12:5:38 | functio ... ext) {} | src/route.js:4:1:5:39 | router. ... xt) {}) | true |
+| src/routesetups.js:4:13:4:13 | h | src/routesetups.js:3:1:4:14 | express ... ('', h) | true |
+| src/routesetups.js:5:11:5:11 | h | src/routesetups.js:3:1:5:12 | express ... ('', h) | true |
+| src/routesetups.js:8:11:8:11 | h | src/routesetups.js:8:1:8:12 | app.error(h) | true |
+| src/routesetups.js:12:15:12:15 | h | src/routesetups.js:12:1:12:16 | root.post('', h) | true |
 | src/subrouter.js:4:19:4:25 | protect | src/subrouter.js:4:1:4:26 | app.use ... rotect) | false |
 | src/subrouter.js:5:14:5:28 | makeSubRouter() | src/subrouter.js:5:1:5:29 | app.use ... uter()) | false |
 | src/subrouter.js:9:27:9:34 | handler1 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | true |
@@ -517,6 +529,7 @@ test_appCreation
 | src/express4.js:2:11:2:19 | express() |
 | src/express.js:2:11:2:19 | express() |
 | src/responseExprs.js:2:11:2:19 | express() |
+| src/routesetups.js:7:11:7:32 | express ... erver() |
 | src/subrouter.js:2:11:2:19 | express() |
 test_RouteSetup_getRequestMethod
 | src/csurf-example.js:20:1:23:2 | app.get ... ) })\\n}) | GET |
@@ -538,11 +551,56 @@ test_RouteSetup_getRequestMethod
 | src/responseExprs.js:10:1:12:2 | app.get ... es3;\\n}) | GET |
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | GET |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | POST |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | GET |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | POST |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | POST |
 | src/subrouter.js:10:3:10:41 | router. ... ndler2) | POST |
 test_RouteExpr
+| src/auth.js:4:1:4:53 | app.use ... d' }})) | src/auth.js:1:13:1:32 | require('express')() |
+| src/csurf-example.js:13:1:13:20 | app.use('/api', api) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:16:1:16:51 | app.use ... lse })) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:17:1:17:23 | app.use ... rser()) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:18:1:18:31 | app.use ... rue })) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:20:1:23:2 | app.get ... ) })\\n}) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:25:1:27:2 | app.pos ... re')\\n}) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:32:3:34:4 | router. ... ')\\n  }) | src/csurf-example.js:30:16:30:35 | new express.Router() |
+| src/csurf-example.js:39:1:39:48 | app.get ... es) {}) | src/csurf-example.js:7:11:7:19 | express() |
+| src/csurf-example.js:40:1:40:49 | app.pos ... es) {}) | src/csurf-example.js:7:11:7:19 | express() |
+| src/express2.js:2:14:2:23 | e.Router() | src/express2.js:2:14:2:23 | e.Router() |
+| src/express2.js:3:1:3:56 | router. ...  res }) | src/express2.js:2:14:2:23 | e.Router() |
+| src/express2.js:3:1:4:77 | router. ... sult }) | src/express2.js:2:14:2:23 | e.Router() |
+| src/express2.js:6:1:6:15 | app.use(router) | src/express2.js:5:11:5:13 | e() |
+| src/express3.js:4:1:7:2 | app.get ... l");\\n}) | src/express3.js:2:11:2:19 | express() |
+| src/express3.js:12:1:12:21 | app.use ... dler()) | src/express3.js:2:11:2:19 | express() |
+| src/express4.js:4:1:6:2 | app.get ... ery;\\n}) | src/express4.js:2:11:2:19 | express() |
+| src/express.js:4:1:9:2 | app.get ... es);\\n}) | src/express.js:2:11:2:19 | express() |
+| src/express.js:16:3:18:4 | router. ... );\\n  }) | src/express.js:2:11:2:19 | express() |
+| src/express.js:22:1:32:2 | app.pos ... r');\\n}) | src/express.js:2:11:2:19 | express() |
+| src/express.js:34:1:34:53 | app.get ... andler) | src/express.js:2:11:2:19 | express() |
+| src/express.js:39:1:39:21 | app.use ... dler()) | src/express.js:2:11:2:19 | express() |
+| src/express.js:44:1:44:26 | app.use ... dler()) | src/express.js:2:11:2:19 | express() |
+| src/express.js:46:1:51:2 | app.pos ... me];\\n}) | src/express.js:2:11:2:19 | express() |
+| src/responseExprs.js:4:1:6:2 | app.get ... res1\\n}) | src/responseExprs.js:2:11:2:19 | express() |
+| src/responseExprs.js:7:1:9:2 | app.get ... es2;\\n}) | src/responseExprs.js:2:11:2:19 | express() |
+| src/responseExprs.js:10:1:12:2 | app.get ... es3;\\n}) | src/responseExprs.js:2:11:2:19 | express() |
+| src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | src/responseExprs.js:2:11:2:19 | express() |
+| src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | src/responseExprs.js:2:11:2:19 | express() |
+| src/route.js:2:14:2:29 | express.Router() | src/route.js:2:14:2:29 | express.Router() |
 | src/route.js:4:1:4:31 | router. ... er_id') | src/route.js:2:14:2:29 | express.Router() |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | src/route.js:2:14:2:29 | express.Router() |
+| src/routesetups.js:3:1:3:16 | express.Router() | src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:7:11:7:32 | express ... erver() | src/routesetups.js:7:11:7:32 | express ... erver() |
+| src/routesetups.js:8:1:8:12 | app.error(h) | src/routesetups.js:7:11:7:32 | express ... erver() |
+| src/routesetups.js:10:14:10:29 | express.Router() | src/routesetups.js:10:14:10:29 | express.Router() |
+| src/routesetups.js:11:12:11:28 | router.route('/') | src/routesetups.js:10:14:10:29 | express.Router() |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:10:14:10:29 | express.Router() |
+| src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:2:11:2:19 | express() |
+| src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:2:11:2:19 | express() |
+| src/subrouter.js:8:16:8:31 | express.Router() | src/subrouter.js:8:16:8:31 | express.Router() |
+| src/subrouter.js:9:3:9:35 | router. ... ndler1) | src/subrouter.js:8:16:8:31 | express.Router() |
+| src/subrouter.js:10:3:10:41 | router. ... ndler2) | src/subrouter.js:8:16:8:31 | express.Router() |
 test_RouteHandler_getAResponseExpr
 | src/csurf-example.js:20:18:23:1 | functio ... () })\\n} | src/csurf-example.js:22:3:22:5 | res |
 | src/csurf-example.js:25:22:27:1 | functio ... ere')\\n} | src/csurf-example.js:26:3:26:5 | res |
@@ -727,6 +785,10 @@ test_RouteSetup_getARouteHandler
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | src/responseExprs.js:13:23:15:1 | functio ... res4;\\n} |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | src/responseExprs.js:16:30:42:1 | functio ...     }\\n} |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | src/route.js:5:12:5:38 | functio ... ext) {} |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | src/routesetups.js:4:13:4:13 | h |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | src/routesetups.js:5:11:5:11 | h |
+| src/routesetups.js:8:1:8:12 | app.error(h) | src/routesetups.js:8:11:8:11 | h |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:12:15:12:15 | h |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:4:19:4:25 | protect |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:5:14:5:28 | makeSubRouter() |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:8:16:8:31 | express.Router() |
@@ -765,6 +827,9 @@ test_isRouterCreation
 | src/express.js:2:11:2:19 | express() |
 | src/responseExprs.js:2:11:2:19 | express() |
 | src/route.js:2:14:2:29 | express.Router() |
+| src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:7:11:7:32 | express ... erver() |
+| src/routesetups.js:10:14:10:29 | express.Router() |
 | src/subrouter.js:2:11:2:19 | express() |
 | src/subrouter.js:8:16:8:31 | express.Router() |
 test_RouteSetup_getRouteHandlerExpr
@@ -797,6 +862,10 @@ test_RouteSetup_getRouteHandlerExpr
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | 0 | src/responseExprs.js:13:23:15:1 | functio ... res4;\\n} |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | 0 | src/responseExprs.js:16:30:42:1 | functio ...     }\\n} |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | 0 | src/route.js:5:12:5:38 | functio ... ext) {} |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | 0 | src/routesetups.js:4:13:4:13 | h |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | 0 | src/routesetups.js:5:11:5:11 | h |
+| src/routesetups.js:8:1:8:12 | app.error(h) | 0 | src/routesetups.js:8:11:8:11 | h |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | 0 | src/routesetups.js:12:15:12:15 | h |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | 0 | src/subrouter.js:4:19:4:25 | protect |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | 0 | src/subrouter.js:5:14:5:28 | makeSubRouter() |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | 0 | src/subrouter.js:9:27:9:34 | handler1 |
@@ -813,6 +882,9 @@ test_RouterDefinition_RouterDefinition
 | src/express.js:2:11:2:19 | express() |
 | src/responseExprs.js:2:11:2:19 | express() |
 | src/route.js:2:14:2:29 | express.Router() |
+| src/routesetups.js:3:1:3:16 | express.Router() |
+| src/routesetups.js:7:11:7:32 | express ... erver() |
+| src/routesetups.js:10:14:10:29 | express.Router() |
 | src/subrouter.js:2:11:2:19 | express() |
 | src/subrouter.js:8:16:8:31 | express.Router() |
 test_RouteHandler_getARequestBodyAccess
@@ -878,6 +950,10 @@ test_RouteSetup_getARouteHandlerExpr
 | src/responseExprs.js:13:1:15:2 | app.get ... es4;\\n}) | src/responseExprs.js:13:23:15:1 | functio ... res4;\\n} |
 | src/responseExprs.js:16:1:42:2 | app.pos ...    }\\n}) | src/responseExprs.js:16:30:42:1 | functio ...     }\\n} |
 | src/route.js:4:1:5:39 | router. ... xt) {}) | src/route.js:5:12:5:38 | functio ... ext) {} |
+| src/routesetups.js:3:1:4:14 | express ... ('', h) | src/routesetups.js:4:13:4:13 | h |
+| src/routesetups.js:3:1:5:12 | express ... ('', h) | src/routesetups.js:5:11:5:11 | h |
+| src/routesetups.js:8:1:8:12 | app.error(h) | src/routesetups.js:8:11:8:11 | h |
+| src/routesetups.js:12:1:12:16 | root.post('', h) | src/routesetups.js:12:15:12:15 | h |
 | src/subrouter.js:4:1:4:26 | app.use ... rotect) | src/subrouter.js:4:19:4:25 | protect |
 | src/subrouter.js:5:1:5:29 | app.use ... uter()) | src/subrouter.js:5:14:5:28 | makeSubRouter() |
 | src/subrouter.js:9:3:9:35 | router. ... ndler1) | src/subrouter.js:9:27:9:34 | handler1 |