add basic support for indirect route handlers

Author: erik-krogh

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

@@ -46,7 +46,12 @@ private DataFlow::SourceNode getARouteUsingCookies(DataFlow::TypeTracker t) {
   t.start() and
   isRouteHandlerUsingCookies(result)
   or
-  exists(DataFlow::TypeTracker t2 | result = getARouteUsingCookies(t2).track(t2, t))
+  exists(DataFlow::TypeTracker t2, DataFlow::SourceNode pred | pred = getARouteUsingCookies(t2) |
+    result = pred.track(t2, t)
+    or
+    t = t2 and
+    Express::routeHandlerStep(pred, result)
+  )
 }
 
 /** Gets a data flow node referring to a route handler that uses cookies. */

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

@@ -72,6 +72,27 @@ module Express {
     result = "del"
   }
 
+  /**
+   * 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) {
+    // 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 |
+      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))
+        )
+      )
+    )
+    or
+    // a container containing route-handlers.
+    exists(HTTP::RouteHandlerCandidateContainer container | pred = container.getRouteHandler(succ))
+  }
+
   /**
    * A call to an Express router method that sets up a route.
    */
@@ -125,9 +146,7 @@ module Express {
       exists(DataFlow::TypeBackTracker t2, DataFlow::SourceNode succ | succ = getARouteHandler(t2) |
         result = succ.backtrack(t2, t)
         or
-        exists(HTTP::RouteHandlerCandidateContainer container |
-          result = container.getRouteHandler(succ)
-        ) and
+        routeHandlerStep(result, succ) and
         t = t2
       )
     }

javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddleware.expected

@@ -1,4 +1,5 @@
 | MissingCsrfMiddlewareBad.js:7:9:7:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:10:26:12:1 | functio ... il"];\\n} | here |
+| MissingCsrfMiddlewareBad.js:17:13:17:26 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | MissingCsrfMiddlewareBad.js:25:30:27:6 | errorCa ... \\n    }) | here |
 | csurf_api_example.js:42:37:42:50 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_api_example.js:42:53:45:3 | functio ... e')\\n  } | here |
 | csurf_example.js:18:9:18:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | csurf_example.js:31:40:34:1 | functio ... sed')\\n} | here |
 | lusca_example.js:9:9:9:22 | cookieParser() | This cookie middleware is serving a request handler $@ without CSRF protection. | lusca_example.js:26:42:29:1 | functio ... sed')\\n} | here |

javascript/ql/test/query-tests/Security/CWE-352/MissingCsrfMiddlewareBad.js

@@ -1,12 +1,28 @@
-var express = require('express')
-var cookieParser = require('cookie-parser')
-var passport = require('passport')
+var express = require('express');
+var cookieParser = require('cookie-parser');
+var passport = require('passport');
 
-var app = express()
+var app = express();
 
-app.use(cookieParser())
-app.use(passport.authorize({ session: true }))
+app.use(cookieParser());
+app.use(passport.authorize({ session: true }));
 
 app.post('/changeEmail', function (req, res) {
     let newEmail = req.cookies["newEmail"];
+});
+
+(function () {
+    var app = express();
+
+    app.use(cookieParser());
+    app.use(passport.authorize({ session: true }));
+
+    const errorCatch = (fn) =>
+        (req, res, next) => {
+            fn(req, res, next).catch((e) => console.log("Caught " + e));
+        };
+
+    app.post('/changeEmail', errorCatch(async function (req, res) {
+        let newEmail = req.cookies["newEmail"];
+    }));
 })