Merge pull request #602 from esben-semmle/js/additional-route-handlers-from-context

Approved by xiemaisi

Author: semmle-qlci

change-notes/1.20/analysis-javascript.md

@@ -2,6 +2,9 @@
 
 ## General improvements
 
+* Support for popular libraries has been improved. Consequently, queries may produce more results on code bases that use the following features:
+  - servers, for example [hapi](https://hapijs.com/)
+
 ## New queries
 
 | **Query**                                     | **Tags**                                             | **Purpose**                                                                                                                                                                 |

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

@@ -32,9 +32,7 @@ module ConnectExpressShared {
    *
    * For example, this could be the function `function(req, res, next){...}`.
    */
-  class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate, DataFlow::FunctionNode {
-
-    override Function astNode;
+  class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate {
 
     RouteHandlerCandidate() {
       exists (string request, string response, string next, string error |

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

@@ -194,26 +194,83 @@ module Hapi {
    */
   class RouteSetup extends MethodCallExpr, HTTP::Servers::StandardRouteSetup {
     ServerDefinition server;
-    string methodName;
+    Expr handler;
 
     RouteSetup() {
       server.flowsTo(getReceiver()) and
-      methodName = getMethodName() and
-      (methodName = "route" or methodName = "ext")
+      (
+        // server.route({ handler: fun })
+        getMethodName() = "route" and
+        hasOptionArgument(0, "handler", handler)
+        or
+        // server.ext('/', fun)
+        getMethodName() = "ext" and
+        handler = getArgument(1)
+      )
     }
 
     override DataFlow::SourceNode getARouteHandler() {
-      // server.route({ handler: fun })
-      methodName = "route" and
-      result.flowsToExpr(any(Expr e | hasOptionArgument(0, "handler", e)))
-      or
-      // server.ext('/', fun)
-      methodName = "ext" and
-      result.flowsToExpr(getArgument(1))
+      result.(DataFlow::SourceNode).flowsTo(handler.flow()) or
+      result.(DataFlow::TrackedNode).flowsTo(handler.flow())
+    }
+
+    Expr getRouteHandlerExpr() {
+      result = handler
     }
 
     override Expr getServer() {
       result = server
     }
   }
+
+  /**
+   * A function that looks like a Hapi route handler.
+   *
+   * For example, this could be the function `function(request, h){...}`.
+   */
+  class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate {
+
+    RouteHandlerCandidate() {
+      exists (string request, string responseToolkit |
+        (request = "request" or request = "req") and
+        responseToolkit = "h" and
+        // heuristic: parameter names match the Hapi documentation
+        astNode.getNumParameter() = 2 and
+        astNode.getParameter(0).getName() = request and
+        astNode.getParameter(1).getName() = responseToolkit |
+        not (
+          // heuristic: is not invoked (Hapi invokes this at a call site we cannot reason precisely about)
+          exists(DataFlow::InvokeNode cs | cs.getACallee() = astNode)
+        )
+      )
+    }
+  }
+
+  /**
+   * Tracking for `RouteHandlerCandidate`.
+   */
+  private class TrackedRouteHandlerCandidate extends DataFlow::TrackedNode {
+
+    TrackedRouteHandlerCandidate() {
+      this instanceof RouteHandlerCandidate
+    }
+
+  }
+
+  /**
+   * A function that looks like a Hapi route handler and flows to a route setup.
+   */
+  private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler, HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
+
+    override Function astNode;
+
+    TrackedRouteHandlerCandidateWithSetup() {
+      exists(TrackedRouteHandlerCandidate tracked |
+        tracked.flowsTo(any(RouteSetup s).getRouteHandlerExpr().flow()) and
+        this = tracked
+      )
+    }
+
+  }
+
 }

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

@@ -597,9 +597,7 @@ module NodeJSLib {
    *
    * For example, this could be the function `function(req, res){...}`.
    */
-  class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate, DataFlow::FunctionNode {
-
-    override Function astNode;
+  class RouteHandlerCandidate extends HTTP::RouteHandlerCandidate {
 
     RouteHandlerCandidate() {
       exists (string request, string response |

javascript/ql/src/semmle/javascript/heuristics/AdditionalRouteHandlers.qll

@@ -8,7 +8,7 @@ import javascript
 private import semmle.javascript.frameworks.ConnectExpressShared
 
 /**
- * Adds `NodeJSLib::RouteHandlerCandidate` to the extent of `NodeJSLib::RouteHandler`.
+ * Add `NodeJSLib::RouteHandlerCandidate` to the extent of `NodeJSLib::RouteHandler`.
  */
 private class PromotedNodeJSLibCandidate extends NodeJSLib::RouteHandler, HTTP::Servers::StandardRouteHandler {
 
@@ -19,7 +19,18 @@ private class PromotedNodeJSLibCandidate extends NodeJSLib::RouteHandler, HTTP::
 }
 
 /**
- * Adds `ConnectExpressShared::RouteHandlerCandidate` to the extent of `Express::RouteHandler`.
+ * Add `Hapi::RouteHandlerCandidate` to the extent of `Hapi::RouteHandler`.
+ */
+private class PromotedHapiCandidate extends Hapi::RouteHandler, HTTP::Servers::StandardRouteHandler {
+
+    PromotedHapiCandidate() {
+      this instanceof Hapi::RouteHandlerCandidate
+    }
+
+}
+
+/**
+ * Add `ConnectExpressShared::RouteHandlerCandidate` to the extent of `Express::RouteHandler`.
  */
 private class PromotedExpressCandidate extends Express::RouteHandler, HTTP::Servers::StandardRouteHandler {
 
@@ -34,7 +45,7 @@ private class PromotedExpressCandidate extends Express::RouteHandler, HTTP::Serv
 }
 
 /**
- * Adds `ConnectExpressShared::RouteHandlerCandidate` to the extent of `Connect::RouteHandler`.
+ * Add `ConnectExpressShared::RouteHandlerCandidate` to the extent of `Connect::RouteHandler`.
  */
 private class PromotedConnectCandidate extends Connect::RouteHandler, HTTP::Servers::StandardRouteHandler {
 

javascript/ql/test/library-tests/frameworks/HTTP-heuristics/RouteHandlerCandidate.expected

@@ -15,6 +15,7 @@
 | src/exported-middleware-attacher.js:2:13:2:32 | function(req, res){} |
 | src/handler-in-property.js:5:14:5:33 | function(req, res){} |
 | src/handler-in-property.js:12:18:12:37 | function(req, res){} |
+| src/hapi.js:1:1:1:30 | functio ... t, h){} |
 | src/iterated-handlers.js:4:2:4:22 | functio ...  res){} |
 | src/middleware-attacher-getter.js:4:17:4:36 | function(req, res){} |
 | src/middleware-attacher-getter.js:19:19:19:38 | function(req, res){} |

javascript/ql/test/library-tests/frameworks/HTTP-heuristics/UnpromotedRouteHandlerCandidate.expected

@@ -1,5 +1,6 @@
 | src/bound-handler.js:4:1:4:28 | functio ...  res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/bound-handler.js:9:12:9:31 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
+| src/hapi.js:1:1:1:30 | functio ... t, h){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/iterated-handlers.js:4:2:4:22 | functio ...  res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/middleware-attacher-getter.js:29:32:29:51 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |
 | src/route-objects.js:7:19:7:38 | function(req, res){} | A `RouteHandlerCandidate` that did not get promoted to `RouteHandler`, and it is not used in a `RouteSetupCandidate`. |

javascript/ql/test/library-tests/frameworks/HTTP-heuristics/src/hapi.js

@@ -0,0 +1 @@
+function handler(request, h){}

javascript/ql/test/library-tests/frameworks/hapi/RouteHandler.expected

@@ -2,3 +2,4 @@
 | src/hapi.js:13:14:15:5 | functio ... n\\n    } | src/hapi.js:4:15:4:31 | new Hapi.Server() |
 | src/hapi.js:17:30:18:1 | functio ... ndler\\n} | src/hapi.js:4:15:4:31 | new Hapi.Server() |
 | src/hapi.js:20:1:27:1 | functio ... oken;\\n} | src/hapi.js:4:15:4:31 | new Hapi.Server() |
+| src/hapi.js:34:12:34:30 | function (req, h){} | src/hapi.js:4:15:4:31 | new Hapi.Server() |

javascript/ql/test/library-tests/frameworks/hapi/RouteSetup.expected

@@ -2,3 +2,4 @@
 | src/hapi.js:12:1:15:7 | server2 ...     }}) |
 | src/hapi.js:17:1:18:2 | server2 ... dler\\n}) |
 | src/hapi.js:29:1:29:20 | server2.route(route) |
+| src/hapi.js:36:1:36:38 | server2 ... ler()}) |