Merge pull request #12956 from asgerf/js/express-array-routes

JS: Properly recognise Express middlewares in an array

Author: asgerf

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

@@ -164,9 +164,9 @@ module Express {
      */
     DataFlow::Node getRouteHandlerNode(int index) {
       // The first argument is a URI pattern if it is a string. If it could possibly be
-      // a function, we consider it to be a route handler, otherwise a URI pattern.
+      // a non-string value, we consider it to be a route handler, otherwise a URI pattern.
       exists(AnalyzedNode firstArg | firstArg = this.getArgument(0).analyze() |
-        if firstArg.getAType() = TTFunction()
+        if firstArg.getAType() != TTString()
         then result = this.getArgument(index)
         else (
           index >= 0 and result = this.getArgument(index + 1)
@@ -215,6 +215,10 @@ module Express {
         or
         Http::routeHandlerStep(result, succ) and
         t = t2
+        or
+        DataFlow::SharedFlowStep::storeStep(result.getALocalUse(), succ,
+          DataFlow::PseudoProperties::arrayElement()) and
+        t = t2.continue()
       )
     }
 
@@ -510,21 +514,6 @@ module Express {
     }
   }
 
-  /**
-   * Holds if `call` is a chainable method call on the response object of `handler`.
-   */
-  private predicate isChainableResponseMethodCall(
-    RouteHandler handler, DataFlow::MethodCallNode call
-  ) {
-    exists(string name | call.calls(handler.getAResponseNode(), name) |
-      name =
-        [
-          "append", "attachment", "location", "send", "sendStatus", "set", "status", "type", "vary",
-          "clearCookie", "contentType", "cookie", "format", "header", "json", "jsonp", "links"
-        ]
-    )
-  }
-
   /** An Express response source. */
   abstract class ResponseSource extends Http::Servers::ResponseSource { }
 
@@ -535,11 +524,7 @@ module Express {
   private class ExplicitResponseSource extends ResponseSource {
     RouteHandler rh;
 
-    ExplicitResponseSource() {
-      this = rh.getResponseParameter()
-      or
-      isChainableResponseMethodCall(rh, this)
-    }
+    ExplicitResponseSource() { this = rh.getResponseParameter() }
 
     /**
      * Gets the route handler that provides this response.
@@ -556,6 +541,22 @@ module Express {
     override RouteHandler getRouteHandler() { none() } // Not known.
   }
 
+  private class ChainedResponse extends ResponseSource {
+    private ResponseSource base;
+
+    ChainedResponse() {
+      this =
+        base.ref()
+            .getAMethodCall([
+                "append", "attachment", "location", "send", "sendStatus", "set", "status", "type",
+                "vary", "clearCookie", "contentType", "cookie", "format", "header", "json", "jsonp",
+                "links"
+              ])
+    }
+
+    override Http::RouteHandler getRouteHandler() { result = base.getRouteHandler() }
+  }
+
   /** An Express request source. */
   abstract class RequestSource extends Http::Servers::RequestSource { }
 
@@ -777,12 +778,12 @@ module Express {
   /**
    * Holds if `e` is an HTTP request object.
    */
-  predicate isRequest(DataFlow::Node e) { any(RouteHandler rh).getARequestNode() = e }
+  predicate isRequest(DataFlow::Node e) { any(RequestSource src).ref().flowsTo(e) }
 
   /**
    * Holds if `e` is an HTTP response object.
    */
-  predicate isResponse(DataFlow::Node e) { any(RouteHandler rh).getAResponseNode() = e }
+  predicate isResponse(DataFlow::Node e) { any(ResponseSource src).ref().flowsTo(e) }
 
   /**
    * An access to the HTTP request body.

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

@@ -1,7 +1,7 @@
 var express = require('express');
 var app = express();
 
-app.get('/some/path', function(req, res) {  
+app.get('/some/path', function(req, res) {
   res.header(req.param("header"), req.param("val"));
   res.send("val");
 });
@@ -10,3 +10,12 @@ function getHandler() {
     return function (req, res){}
 }
 app.use(getHandler());
+
+function getHandler2() {
+  return function (req, res){}
+}
+app.use([getHandler2()]);
+
+function handler3(req, res) {}
+let array = [handler3];
+app.use(array);