simplify the connect model by reusing NodeJSLib::RouteHandler

erik-krogh · erik-krogh · commit c6399dbdf430 · 2021-08-31T14:23:23.000+02:00
diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Connect.qll b/javascript/ql/lib/semmle/javascript/frameworks/Connect.qll
@@ -24,7 +24,8 @@ module Connect {
    * but support for other kinds of route handlers can be added by implementing
    * additional subclasses of this class.
    */
-  abstract class RouteHandler extends HTTP::Servers::StandardRouteHandler, DataFlow::ValueNode {
+  abstract class RouteHandler extends HTTP::Servers::StandardRouteHandler, NodeJSLib::RouteHandler,
+    DataFlow::ValueNode {
     /**
      * Gets the parameter of kind `kind` of this route handler.
      *
@@ -35,12 +36,12 @@ module Connect {
     /**
      * Gets the parameter of the route handler that contains the request object.
      */
-    Parameter getRequestParameter() { result = getRouteHandlerParameter("request") }
+    override Parameter getRequestParameter() { result = getRouteHandlerParameter("request") }
 
     /**
      * Gets the parameter of the route handler that contains the response object.
      */
-    Parameter getResponseParameter() { result = getRouteHandlerParameter("response") }
+    override Parameter getResponseParameter() { result = getRouteHandlerParameter("response") }
   }
 
   /**
@@ -56,50 +57,6 @@ module Connect {
     }
   }
 
-  /**
-   * A Connect response source, that is, the response parameter of a
-   * route handler.
-   */
-  private class ResponseSource extends HTTP::Servers::ResponseSource {
-    RouteHandler rh;
-
-    ResponseSource() { this = DataFlow::parameterNode(rh.getResponseParameter()) }
-
-    /**
-     * Gets the route handler that provides this response.
-     */
-    override RouteHandler getRouteHandler() { result = rh }
-  }
-
-  /**
-   * A Connect request source, that is, the request parameter of a
-   * route handler.
-   */
-  private class RequestSource extends HTTP::Servers::RequestSource {
-    RouteHandler rh;
-
-    RequestSource() { this = DataFlow::parameterNode(rh.getRequestParameter()) }
-
-    /**
-     * Gets the route handler that handles this request.
-     */
-    override RouteHandler getRouteHandler() { result = rh }
-  }
-
-  /**
-   * A Node.js HTTP response provided by Connect.
-   */
-  class ResponseExpr extends NodeJSLib::ResponseExpr {
-    ResponseExpr() { src instanceof ResponseSource }
-  }
-
-  /**
-   * A Node.js HTTP request provided by Connect.
-   */
-  class RequestExpr extends NodeJSLib::RequestExpr {
-    RequestExpr() { src instanceof RequestSource }
-  }
-
   /**
    * A call to a Connect method that sets up a route.
    */
@@ -156,10 +113,11 @@ module Connect {
    * An access to a user-controlled Connect request input.
    */
   private class RequestInputAccess extends HTTP::RequestInputAccess {
-    RequestExpr request;
+    NodeJSLib::RequestExpr request;
     string kind;
 
     RequestInputAccess() {
+      request.getRouteHandler() instanceof StandardRouteHandler and
       exists(PropAccess cookies |
         // `req.cookies.get(<name>)`
         kind = "cookie" and
@@ -172,33 +130,4 @@ module Connect {
 
     override string getKind() { result = kind }
   }
-
-  /**
-   * A function that flows to a route setup.
-   */
-  private class TrackedRouteHandlerCandidateWithSetup extends RouteHandler,
-    HTTP::Servers::StandardRouteHandler, DataFlow::FunctionNode {
-    TrackedRouteHandlerCandidateWithSetup() { this = any(RouteSetup s).getARouteHandler() }
-
-    override Parameter getRouteHandlerParameter(string kind) {
-      result = getRouteHandlerParameter(astNode, kind)
-    }
-  }
-
-  /**
-   * A call that looks like a route setup on a Connect server.
-   *
-   * For example, this could be the call `router.use(handler)` where
-   * it is unknown if `router` is a Connect router.
-   */
-  class RouteSetupCandidate extends HTTP::RouteSetupCandidate, DataFlow::MethodCallNode {
-    DataFlow::ValueNode routeHandlerArg;
-
-    RouteSetupCandidate() {
-      getMethodName() = "use" and
-      routeHandlerArg = getAnArgument()
-    }
-
-    override DataFlow::ValueNode getARouteHandlerArg() { result = routeHandlerArg }
-  }
 }
diff --git a/javascript/ql/test/library-tests/frameworks/connect/RequestExpr.qll b/javascript/ql/test/library-tests/frameworks/connect/RequestExpr.qll
@@ -1,5 +1,5 @@
 import javascript
 
-query predicate test_RequestExpr(Connect::RequestExpr e, HTTP::RouteHandler res) {
+query predicate test_RequestExpr(HTTP::RequestExpr e, HTTP::RouteHandler res) {
   res = e.getRouteHandler()
 }
diff --git a/javascript/ql/test/library-tests/frameworks/connect/ResponseExpr.qll b/javascript/ql/test/library-tests/frameworks/connect/ResponseExpr.qll
@@ -1,5 +1,5 @@
 import javascript
 
-query predicate test_ResponseExpr(Connect::ResponseExpr e, HTTP::RouteHandler res) {
+query predicate test_ResponseExpr(HTTP::ResponseExpr e, HTTP::RouteHandler res) {
   res = e.getRouteHandler()
 }

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`import javascript`
`2`	`2`
`3`		`-query predicate test_RequestExpr(Connect::RequestExpr e, HTTP::RouteHandler res) {`
	`3`	`+query predicate test_RequestExpr(HTTP::RequestExpr e, HTTP::RouteHandler res) {`
`4`	`4`	`res = e.getRouteHandler()`
`5`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`import javascript`
`2`	`2`
`3`		`-query predicate test_ResponseExpr(Connect::ResponseExpr e, HTTP::RouteHandler res) {`
	`3`	`+query predicate test_ResponseExpr(HTTP::ResponseExpr e, HTTP::RouteHandler res) {`
`4`	`4`	`res = e.getRouteHandler()`
`5`	`5`	`}`