change RouteSetup to a DataFlow::Node

Author: erik-krogh

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

@@ -59,17 +59,17 @@ module Connect {
   /**
    * A call to a Connect method that sets up a route.
    */
-  class RouteSetup extends MethodCallExpr, HTTP::Servers::StandardRouteSetup {
+  class RouteSetup extends DataFlow::MethodCallNode, HTTP::Servers::StandardRouteSetup {
     ServerDefinition server;
 
     RouteSetup() {
       getMethodName() = "use" and
       (
         // app.use(fun)
-        server.ref().flowsToExpr(getReceiver())
+        server.ref().getAMethodCall() = this
         or
         // app.use(...).use(fun)
-        this.getReceiver().(RouteSetup).getServer() = server.asExpr()
+        this.getReceiver().(RouteSetup).getServer() = server
       )
     }
 
@@ -84,10 +84,10 @@ module Connect {
       exists(DataFlow::TypeBackTracker t2 | result = getARouteHandler(t2).backtrack(t2, t))
     }
 
-    override Expr getServer() { result = server.asExpr() }
+    override DataFlow::Node getServer() { result = server }
 
     /** Gets an argument that represents a route handler being registered. */
-    Expr getARouteHandlerExpr() { result = getAnArgument() }
+    Expr getARouteHandlerExpr() { result = getAnArgument().asExpr() } // TODO: DataFlow::Node
   }
 
   /** An expression that is passed as `basicAuthConnect(<user>, <password>)`. */

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

@@ -49,6 +49,7 @@ module Express {
    * Holds if `e` may refer to a router object.
    */
   private predicate isRouter(Expr e) {
+    // TODO: DataFlow::Node
     isRouter(e, _)
     or
     e.getType().hasUnderlyingType("express", "Router")
@@ -89,7 +90,7 @@ module Express {
   }
 
   private class RoutingTreeSetup extends Routing::RouteSetup::MethodCall {
-    RoutingTreeSetup() { this.asExpr() instanceof RouteSetup }
+    RoutingTreeSetup() { this instanceof RouteSetup }
 
     override string getRelativePath() {
       not this.getMethodName() = "param" and // do not treat parameter name as a path
@@ -140,17 +141,17 @@ module Express {
   /**
    * A call to an Express router method that sets up a route.
    */
-  class RouteSetup extends HTTP::Servers::StandardRouteSetup, MethodCallExpr {
+  class RouteSetup extends HTTP::Servers::StandardRouteSetup, DataFlow::MethodCallNode {
     RouteSetup() {
-      isRouter(this.getReceiver()) and
+      isRouter(this.getReceiver().asExpr()) and
       this.getMethodName() = routeSetupMethodName()
     }
 
     /** Gets the path associated with the route. */
     string getPath() { this.getArgument(0).mayHaveStringValue(result) }
 
     /** Gets the router on which handlers are being registered. */
-    RouterDefinition getRouter() { isRouter(this.getReceiver(), result) }
+    RouterDefinition getRouter() { isRouter(this.getReceiver().asExpr(), result) }
 
     /** Holds if this is a call `use`, such as `app.use(handler)`. */
     predicate isUseCall() { this.getMethodName() = "use" }
@@ -162,13 +163,14 @@ module Express {
      * returned, not its dataflow source.
      */
     Expr getRouteHandlerExpr(int index) {
+      // TODO: DataFlow::Node
       // 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.
       exists(AnalyzedNode firstArg | firstArg = this.getArgument(0).analyze() |
         if firstArg.getAType() = TTFunction()
-        then result = this.getArgument(index)
+        then result = this.getArgument(index).asExpr()
         else (
-          index >= 0 and result = this.getArgument(index + 1)
+          index >= 0 and result = this.getArgument(index + 1).asExpr()
         )
       )
     }
@@ -199,9 +201,8 @@ module Express {
       )
     }
 
-    override Expr getServer() {
-      any(DataFlow::Node n | n.asExpr() = result).(Application).getARouteHandler() =
-        this.getARouteHandler()
+    override DataFlow::Node getServer() {
+      result.(Application).getARouteHandler() = this.getARouteHandler()
     }
 
     /**
@@ -236,24 +237,24 @@ module Express {
   /**
    * A call that sets up a Passport router that includes the request object.
    */
-  private class PassportRouteSetup extends HTTP::Servers::StandardRouteSetup, CallExpr {
+  private class PassportRouteSetup extends HTTP::Servers::StandardRouteSetup, DataFlow::CallNode {
     DataFlow::ModuleImportNode importNode;
     DataFlow::FunctionNode callback;
 
     // looks for this pattern: passport.use(new Strategy({passReqToCallback: true}, callback))
     PassportRouteSetup() {
       importNode = DataFlow::moduleImport("passport") and
-      this = importNode.getAMemberCall("use").asExpr() and
+      this = importNode.getAMemberCall("use") and
       exists(DataFlow::NewNode strategy |
-        strategy.flowsToExpr(this.getArgument(0)) and
+        strategy.flowsTo(this.getArgument(0)) and
         strategy.getNumArgument() = 2 and
         // new Strategy({passReqToCallback: true}, ...)
         strategy.getOptionArgument(0, "passReqToCallback").mayHaveBooleanValue(true) and
         callback.flowsTo(strategy.getArgument(1))
       )
     }
 
-    override Expr getServer() { result = importNode.asExpr() }
+    override DataFlow::Node getServer() { result = importNode }
 
     override DataFlow::SourceNode getARouteHandler() { result = callback }
   }
@@ -335,7 +336,8 @@ module Express {
      * same requests.
      */
     Express::RouteHandlerExpr getPreviousMiddleware() {
-      index = 0 and result = setup.getRouter().getMiddlewareStackAt(setup.getAPredecessor())
+      index = 0 and
+      result = setup.getRouter().getMiddlewareStackAt(setup.asExpr().getAPredecessor())
       or
       index > 0 and result = setup.getRouteHandlerExpr(index - 1)
       or
@@ -867,15 +869,15 @@ module Express {
     /**
      * Gets a `RouteSetup` that was used for setting up a route on this router.
      */
-    private RouteSetup getARouteSetup() { this.ref().flowsToExpr(result.getReceiver()) }
+    private RouteSetup getARouteSetup() { this.ref().flowsTo(result.getReceiver()) }
 
     /**
      * Gets a sub-router registered on this router.
      *
      * Example: `router2` for `router1.use(router2)` or `router1.use("/route2", router2)`
      */
     RouterDefinition getASubRouter() {
-      result.ref().flowsToExpr(this.getARouteSetup().getAnArgument())
+      result.ref().flowsTo(this.getARouteSetup().getAnArgument())
     }
 
     /**
@@ -884,7 +886,7 @@ module Express {
      * Example: `fun` for `router1.use(fun)` or `router.use("/route", fun)`
      */
     HTTP::RouteHandler getARouteHandler() {
-      result.(DataFlow::SourceNode).flowsToExpr(this.getARouteSetup().getAnArgument())
+      result.(DataFlow::SourceNode).flowsToExpr(this.getARouteSetup().getAnArgument().asExpr())
     }
 
     /**
@@ -904,10 +906,10 @@ module Express {
      */
     Express::RouteHandlerExpr getMiddlewareStackAt(ControlFlowNode node) {
       if
-        exists(Express::RouteSetup setup | node = setup and setup.getRouter() = this |
+        exists(Express::RouteSetup setup | node = setup.asExpr() and setup.getRouter() = this |
           setup.isUseCall()
         )
-      then result = node.(Express::RouteSetup).getLastRouteHandlerExpr()
+      then result = node.(AST::ValueNode).flow().(Express::RouteSetup).getLastRouteHandlerExpr()
       else result = this.getMiddlewareStackAt(node.getAPredecessor())
     }
 

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

@@ -132,12 +132,12 @@ module Fastify {
   /**
    * A call to a Fastify method that sets up a route.
    */
-  class RouteSetup extends MethodCallExpr, HTTP::Servers::StandardRouteSetup {
+  class RouteSetup extends DataFlow::MethodCallNode, HTTP::Servers::StandardRouteSetup {
     ServerDefinition server;
     string methodName;
 
     RouteSetup() {
-      this = server(server).getAMethodCall(methodName).asExpr() and
+      this = server(server).getAMethodCall(methodName) and
       methodName = ["route", "get", "head", "post", "put", "delete", "options", "patch"]
     }
 
@@ -152,20 +152,19 @@ module Fastify {
       exists(DataFlow::TypeBackTracker t2 | result = this.getARouteHandler(t2).backtrack(t2, t))
     }
 
-    override Expr getServer() { result = server.asExpr() }
+    override DataFlow::SourceNode getServer() { result = server }
 
     /** Gets an argument that represents a route handler being registered. */
     DataFlow::Node getARouteHandlerExpr() {
       if methodName = "route"
-      then
-        result = this.flow().(DataFlow::MethodCallNode).getOptionArgument(0, getNthHandlerName(_))
-      else result = this.getLastArgument().flow()
+      then result = this.getOptionArgument(0, getNthHandlerName(_))
+      else result = this.getLastArgument()
     }
   }
 
   private class ShorthandRoutingTreeSetup extends Routing::RouteSetup::MethodCall {
     ShorthandRoutingTreeSetup() {
-      this.asExpr() instanceof RouteSetup and
+      this instanceof RouteSetup and
       not this.getMethodName() = "route"
     }
 
@@ -183,7 +182,7 @@ module Fastify {
 
   private class FullRoutingTreeSetup extends Routing::RouteSetup::MethodCall {
     FullRoutingTreeSetup() {
-      this.asExpr() instanceof RouteSetup and
+      this instanceof RouteSetup and
       this.getMethodName() = "route"
     }
 
@@ -285,13 +284,7 @@ module Fastify {
    */
   private predicate usesFastifyPlugin(RouteHandler rh, DataFlow::SourceNode plugin) {
     exists(RouteSetup setup |
-      plugin
-          .flowsTo(setup
-                .getServer()
-                .flow()
-                .(DataFlow::SourceNode)
-                .getAMethodCall("register")
-                .getArgument(0)) and // only matches the plugins that apply to all routes
+      plugin.flowsTo(setup.getServer().getAMethodCall("register").getArgument(0)) and // only matches the plugins that apply to all routes
       rh = setup.getARouteHandler()
     )
   }
@@ -301,13 +294,7 @@ module Fastify {
    */
   private predicate usesMiddleware(RouteHandler rh, DataFlow::SourceNode middleware) {
     exists(RouteSetup setup |
-      middleware
-          .flowsTo(setup
-                .getServer()
-                .flow()
-                .(DataFlow::SourceNode)
-                .getAMethodCall("use")
-                .getArgument(0)) and // only matches the middlewares that apply to all routes
+      middleware.flowsTo(setup.getServer().getAMethodCall("use").getArgument(0)) and // only matches the middlewares that apply to all routes
       rh = setup.getARouteHandler()
     )
   }

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

@@ -195,23 +195,21 @@ module Firebase {
     /**
      * A call to a Firebase method that sets up a route.
      */
-    private class RouteSetup extends HTTP::Servers::StandardRouteSetup, CallExpr {
-      RouteSetup() {
-        this = namespace().getAPropertyRead("https").getAMemberCall("onRequest").asExpr()
-      }
+    private class RouteSetup extends HTTP::Servers::StandardRouteSetup, DataFlow::CallNode {
+      RouteSetup() { this = namespace().getAPropertyRead("https").getAMemberCall("onRequest") }
 
       override DataFlow::SourceNode getARouteHandler() {
         result = getARouteHandler(DataFlow::TypeBackTracker::end())
       }
 
       private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
         t.start() and
-        result = getArgument(0).flow().getALocalSource()
+        result = getArgument(0).getALocalSource()
         or
         exists(DataFlow::TypeBackTracker t2 | result = getARouteHandler(t2).backtrack(t2, t))
       }
 
-      override Expr getServer() { none() }
+      override DataFlow::Node getServer() { none() }
     }
 
     /**

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

@@ -242,7 +242,7 @@ module HTTP {
   /**
    * An expression that sets up a route on a server.
    */
-  abstract class RouteSetup extends Expr { } // TODO: DataFlow::Node
+  abstract class RouteSetup extends DataFlow::Node { }
 
   /**
    * An expression that may contain a request object.
@@ -275,9 +275,7 @@ module HTTP {
      * A standard server definition.
      */
     abstract class StandardServerDefinition extends ServerDefinition {
-      override RouteHandler getARouteHandler() {
-        result.(StandardRouteHandler).getServer() = this.asExpr()
-      }
+      override RouteHandler getARouteHandler() { result.(StandardRouteHandler).getServer() = this }
 
       private DataFlow::SourceNode ref(DataFlow::TypeTracker t) {
         t.start() and
@@ -308,8 +306,7 @@ module HTTP {
       /**
        * Gets the server this route handler is registered on.
        */
-      Expr getServer() {
-        // TODO: DataFlow::Node
+      DataFlow::Node getServer() {
         exists(StandardRouteSetup setup | setup.getARouteHandler() = this |
           result = setup.getServer()
         )
@@ -414,7 +411,7 @@ module HTTP {
       /**
        * Gets the server on which this route setup sets up routes.
        */
-      abstract Expr getServer(); // TODO: DataFlow::Node
+      abstract DataFlow::Node getServer();
     }
 
     /**

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

@@ -186,16 +186,16 @@ module Hapi {
   /**
    * A call to a Hapi method that sets up a route.
    */
-  class RouteSetup extends MethodCallExpr, HTTP::Servers::StandardRouteSetup {
+  class RouteSetup extends DataFlow::MethodCallNode, HTTP::Servers::StandardRouteSetup {
     ServerDefinition server;
-    Expr handler;
+    DataFlow::Node handler;
 
     RouteSetup() {
-      server.ref().flowsToExpr(getReceiver()) and
+      server.ref().getAMethodCall() = this and
       (
         // server.route({ handler: fun })
         getMethodName() = "route" and
-        hasOptionArgument(0, "handler", handler)
+        getOptionArgument(0, "handler") = handler
         or
         // server.ext('/', fun)
         getMethodName() = "ext" and
@@ -215,11 +215,11 @@ module Hapi {
     }
 
     pragma[noinline]
-    private DataFlow::Node getRouteHandler() { result = handler.flow() }
+    private DataFlow::Node getRouteHandler() { result = handler }
 
-    Expr getRouteHandlerExpr() { result = handler }
+    Expr getRouteHandlerExpr() { result = handler.asExpr() } // TODO: DataFlow::Node
 
-    override Expr getServer() { result = server.asExpr() }
+    override DataFlow::Node getServer() { result = server }
   }
 
   /**

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

@@ -384,24 +384,23 @@ module Koa {
   /**
    * A call to a Koa method that sets up a route.
    */
-  class RouteSetup extends HTTP::Servers::StandardRouteSetup, MethodCallExpr {
+  class RouteSetup extends HTTP::Servers::StandardRouteSetup, DataFlow::MethodCallNode {
     AppDefinition server;
 
     RouteSetup() {
       // app.use(fun)
-      server.ref().flowsToExpr(this.getReceiver()) and
-      this.getMethodName() = "use"
+      server.ref().getAMethodCall("use") = this
     }
 
     override DataFlow::SourceNode getARouteHandler() {
       // `StandardRouteHandler` uses this predicate in it's charpred, so making this predicate return a `RouteHandler` would give an empty recursion.
-      result.flowsToExpr(this.getArgument(0))
+      result.flowsToExpr(this.getArgument(0).asExpr())
       or
       // For the route-handlers that does not depend on this predicate in their charpred.
-      result.(RouteHandler).getARouteHandlerRegistrationObject().flowsToExpr(this.getArgument(0))
+      result.(RouteHandler).getARouteHandlerRegistrationObject().flowsTo(this.getArgument(0))
     }
 
-    override Expr getServer() { result = server.asExpr() }
+    override DataFlow::Node getServer() { result = server }
   }
 
   /**