deprecate RequestExpr and ResponseExpr and use ResponseNode and RequestNode instead

Author: erik-krogh

javascript/ql/experimental/adaptivethreatmodeling/lib/experimental/adaptivethreatmodeling/NosqlInjectionATM.qll

@@ -42,10 +42,10 @@ module SinkEndpointFilter {
       result = "modeled database access"
       or
       // Remove calls to APIs that aren't relevant to NoSQL injection
-      call.getReceiver().asExpr() instanceof HTTP::RequestExpr and
+      call.getReceiver() instanceof HTTP::RequestNode and
       result = "receiver is a HTTP request expression"
       or
-      call.getReceiver().asExpr() instanceof HTTP::ResponseExpr and
+      call.getReceiver() instanceof HTTP::ResponseNode and
       result = "receiver is a HTTP response expression"
     )
     or

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

@@ -108,22 +108,24 @@ module Connect {
     override string getCredentialsKind() { result = kind }
   }
 
-  class RequestExpr = NodeJSLib::RequestExpr;
+  deprecated class RequestExpr = NodeJSLib::RequestExpr;
+
+  class RequestNode = NodeJSLib::RequestNode;
 
   /**
    * An access to a user-controlled Connect request input.
    */
-  private class RequestInputAccess extends HTTP::RequestInputAccess {
-    RequestExpr request;
+  private class RequestInputAccess extends HTTP::RequestInputAccess instanceof DataFlow::MethodCallNode {
+    RequestNode request;
     string kind;
 
     RequestInputAccess() {
       request.getRouteHandler() instanceof StandardRouteHandler and
-      exists(PropAccess cookies |
+      exists(DataFlow::PropRead cookies |
         // `req.cookies.get(<name>)`
         kind = "cookie" and
         cookies.accesses(request, "cookies") and
-        this.asExpr().(MethodCallExpr).calls(cookies, "get")
+        super.calls(cookies, "get")
       )
     }
 

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

@@ -409,22 +409,24 @@ module Express {
      *
      * `kind` is one of: "error", "request", "response", "next", or "parameter".
      */
-    abstract Parameter getRouteHandlerParameter(string kind);
+    abstract Parameter getRouteHandlerParameter(string kind); // TODO: DataFlow::ParameterNode
 
     /**
      * Gets the parameter of the route handler that contains the request object.
      */
-    Parameter getRequestParameter() { result = this.getRouteHandlerParameter("request") }
+    Parameter getRequestParameter() { result = this.getRouteHandlerParameter("request") } // TODO: DataFlow::ParameterNode
 
     /**
      * Gets the parameter of the route handler that contains the response object.
      */
-    Parameter getResponseParameter() { result = this.getRouteHandlerParameter("response") }
+    Parameter getResponseParameter() { result = this.getRouteHandlerParameter("response") } // TODO: DataFlow::ParameterNode
 
     /**
      * Gets a request body access of this handler.
      */
-    Expr getARequestBodyAccess() { result.(PropAccess).accesses(this.getARequestExpr(), "body") }
+    Expr getARequestBodyAccess() {
+      result.(PropAccess).accesses(this.getARequestNode().asExpr(), "body")
+    } // TODO: DataFlow::Node
   }
 
   /**
@@ -448,7 +450,8 @@ module Express {
    * Holds if `call` is a chainable method call on the response object of `handler`.
    */
   private predicate isChainableResponseMethodCall(RouteHandler handler, MethodCallExpr call) {
-    exists(string name | call.calls(handler.getAResponseExpr(), name) |
+    // TODO: DataFlow::MethodCallNode
+    exists(string name | call.calls(handler.getAResponseNode().asExpr(), name) |
       name =
         [
           "append", "attachment", "location", "send", "sendStatus", "set", "status", "type", "vary",
@@ -516,16 +519,32 @@ module Express {
   }
 
   /**
+   * DEPRECATED: Use `ResponseNode` instead.
    * An Express response expression.
    */
-  class ResponseExpr extends NodeJSLib::ResponseExpr {
+  deprecated class ResponseExpr extends NodeJSLib::ResponseExpr {
+    ResponseExpr() { this.flow() instanceof ResponseNode }
+  }
+
+  /**
+   * An Express response expression.
+   */
+  class ResponseNode extends NodeJSLib::ResponseNode {
     override ResponseSource src;
   }
 
+  /**
+   * DEPRECATED: Use `RequestNode` instead.
+   * An Express request expression.
+   */
+  deprecated class RequestExpr extends NodeJSLib::RequestExpr {
+    RequestExpr() { this.flow() instanceof RequestNode }
+  }
+
   /**
    * An Express request expression.
    */
-  class RequestExpr extends NodeJSLib::RequestExpr {
+  class RequestNode extends NodeJSLib::RequestNode {
     override RequestSource src;
   }
 
@@ -679,12 +698,12 @@ module Express {
   /**
    * Holds if `e` is an HTTP request object.
    */
-  predicate isRequest(Expr e) { any(RouteHandler rh).getARequestExpr() = e } // TODO: DataFlow::Node
+  predicate isRequest(Expr e) { any(RouteHandler rh).getARequestNode().asExpr() = e } // TODO: DataFlow::Node
 
   /**
    * Holds if `e` is an HTTP response object.
    */
-  predicate isResponse(Expr e) { any(RouteHandler rh).getAResponseExpr() = e } // TODO: DataFlow::Node
+  predicate isResponse(Expr e) { any(RouteHandler rh).getAResponseNode().asExpr() = e } // TODO: DataFlow::Node
 
   /**
    * An access to the HTTP request body.
@@ -696,9 +715,7 @@ module Express {
   abstract private class HeaderDefinition extends HTTP::Servers::StandardHeaderDefinition {
     HeaderDefinition() { isResponse(this.getReceiver().asExpr()) }
 
-    override RouteHandler getRouteHandler() {
-      this.getReceiver().asExpr() = result.getAResponseExpr()
-    }
+    override RouteHandler getRouteHandler() { this.getReceiver() = result.getAResponseNode() }
   }
 
   /**
@@ -876,9 +893,7 @@ module Express {
      *
      * Example: `router2` for `router1.use(router2)` or `router1.use("/route2", router2)`
      */
-    RouterDefinition getASubRouter() {
-      result.ref().flowsTo(this.getARouteSetup().getAnArgument())
-    }
+    RouterDefinition getASubRouter() { result.ref().flowsTo(this.getARouteSetup().getAnArgument()) }
 
     /**
      * Gets a route handler registered on this router.
@@ -948,7 +963,7 @@ module Express {
     DataFlow::MethodCallNode {
     ResponseSendFileAsFileSystemAccess() {
       exists(string name | name = "sendFile" or name = "sendfile" |
-        this.calls(any(ResponseExpr res).flow(), name)
+        this.calls(any(ResponseNode res), name)
       )
     }
 

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

@@ -208,16 +208,30 @@ module HTTP {
     final Servers::ResponseSource getAResponseSource() { result.getRouteHandler() = this }
 
     /**
+     * DEPRECATED: Use `getARequestNode()` instead.
      * Gets an expression that contains a request object handled
      * by this handler.
      */
-    RequestExpr getARequestExpr() { result.getRouteHandler() = this } // TODO: DataFlow::Node
+    deprecated RequestExpr getARequestExpr() { result.flow() = this.getARequestNode() }
+
+    /**
+     * Gets an expression that contains a request object handled
+     * by this handler.
+     */
+    RequestNode getARequestNode() { result.getRouteHandler() = this }
+
+    /**
+     * DEPRECATED: Use `getAResponseNode()` instead.
+     * Gets an expression that contains a response object provided
+     * by this handler.
+     */
+    deprecated ResponseExpr getAResponseExpr() { result.flow() = this.getAResponseNode() }
 
     /**
      * Gets an expression that contains a response object provided
      * by this handler.
      */
-    ResponseExpr getAResponseExpr() { result.getRouteHandler() = this } // TODO: DataFlow::Node
+    ResponseNode getAResponseNode() { result.getRouteHandler() = this }
   }
 
   /**
@@ -244,26 +258,40 @@ module HTTP {
    */
   abstract class RouteSetup extends DataFlow::Node { }
 
+  /** A dataflow node that may contain a request object. */
+  abstract class RequestNode extends DataFlow::Node {
+    /** Gets the route handler that handles this request. */
+    abstract RouteHandler getRouteHandler();
+  }
+
+  /** An dataflow node that may contain a response object. */
+  abstract class ResponseNode extends DataFlow::Node {
+    /** Gets the route handler that handles this request. */
+    abstract RouteHandler getRouteHandler();
+  }
+
   /**
+   * DEPRECATED: Use `RequestNode` instead.
    * An expression that may contain a request object.
    */
-  abstract class RequestExpr extends Expr {
-    // TODO: DataFlow::Node
+  deprecated class RequestExpr extends Expr {
+    RequestExpr() { this.flow() instanceof ResponseNode }
+
     /**
      * Gets the route handler that handles this request.
      */
-    abstract RouteHandler getRouteHandler();
+    RouteHandler getRouteHandler() { result = this.flow().(ResponseNode).getRouteHandler() }
   }
 
   /**
+   * DEPRECATED: Use `ResponseNode` instead.
    * An expression that may contain a response object.
    */
-  abstract class ResponseExpr extends Expr {
-    // TODO: DataFlow::Node
+  deprecated class ResponseExpr extends Expr {
     /**
      * Gets the route handler that handles this request.
      */
-    abstract RouteHandler getRouteHandler();
+    RouteHandler getRouteHandler() { result = this.flow().(ResponseNode).getRouteHandler() }
   }
 
   /**
@@ -366,25 +394,49 @@ module HTTP {
     /**
      * A request expression arising from a request source.
      */
-    class StandardRequestExpr extends RequestExpr {
+    class StandardRequestNode extends RequestNode {
       RequestSource src;
 
-      StandardRequestExpr() { src.ref().flowsTo(DataFlow::valueNode(this)) }
+      StandardRequestNode() { src.ref().flowsTo(this) }
 
       override RouteHandler getRouteHandler() { result = src.getRouteHandler() }
     }
 
     /**
      * A response expression arising from a response source.
      */
-    class StandardResponseExpr extends ResponseExpr {
+    class StandardResponseNode extends ResponseNode {
       ResponseSource src;
 
-      StandardResponseExpr() { src.ref().flowsTo(DataFlow::valueNode(this)) }
+      StandardResponseNode() { src.ref().flowsTo(this) }
 
       override RouteHandler getRouteHandler() { result = src.getRouteHandler() }
     }
 
+    /**
+     * A request expression arising from a request source.
+     */
+    deprecated class StandardRequestExpr extends RequestExpr {
+      RequestSource src;
+
+      StandardRequestExpr() { src.ref().flowsToExpr(this) }
+
+      override RouteHandler getRouteHandler() { result = src.getRouteHandler() }
+    }
+
+    /**
+     * A response expression arising from a response source.
+     */
+    deprecated class StandardResponseExpr extends ResponseExpr {
+      ResponseSource src;
+
+      StandardResponseExpr() { src.ref().flowsToExpr(this) }
+
+      override RouteHandler getRouteHandler() {
+        result = this.flow().(StandardResponseNode).getRouteHandler()
+      }
+    }
+
     /**
      * A standard header definition.
      */