Merge pull request github#6572 from erik-krogh/live-server

Approved by esbena

Author: codeql-ci

javascript/change-notes/2021-08-30-live-server.md

@@ -0,0 +1,6 @@
+lgtm,codescanning
+* The dataflow libraries now model sources and sinks from the `live-server` library.
+  The model for `connect` has improved to recognize more sources and sinks.
+  Affected packages are
+    [live-server](https://www.npmjs.com/package/live-server),
+    [connect](https://www.npmjs.com/package/connect)

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

@@ -24,7 +24,7 @@ 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 NodeJSLib::RouteHandler, DataFlow::ValueNode {
     /**
      * Gets the parameter of kind `kind` of this route handler.
      *
@@ -35,12 +35,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 +56,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.
    */
@@ -152,6 +108,8 @@ module Connect {
     override string getCredentialsKind() { result = kind }
   }
 
+  class RequestExpr = NodeJSLib::RequestExpr;
+
   /**
    * An access to a user-controlled Connect request input.
    */
@@ -160,6 +118,7 @@ module Connect {
     string kind;
 
     RequestInputAccess() {
+      request.getRouteHandler() instanceof StandardRouteHandler and
       exists(PropAccess cookies |
         // `req.cookies.get(<name>)`
         kind = "cookie" and
@@ -172,33 +131,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 }
-  }
 }

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

@@ -1,6 +1,7 @@
 import semmle.javascript.frameworks.Express
 import semmle.javascript.frameworks.Hapi
 import semmle.javascript.frameworks.Koa
+import semmle.javascript.frameworks.LiveServer
 import semmle.javascript.frameworks.NodeJSLib
 import semmle.javascript.frameworks.Micro
 import semmle.javascript.frameworks.Restify

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

@@ -0,0 +1,59 @@
+/**
+ * Provides classes modelling the [live-server](https://npmjs.com/package/live-server) package.
+ */
+
+import javascript
+private import semmle.javascript.frameworks.ConnectExpressShared::ConnectExpressShared as ConnectExpressShared
+
+private module LiveServer {
+  /**
+   * An expression that imports the live-server package, seen as a server-definition.
+   */
+  class ServerDefinition extends HTTP::Servers::StandardServerDefinition {
+    API::Node imp;
+
+    ServerDefinition() {
+      imp = API::moduleImport("live-server") and
+      this = imp.getAnImmediateUse().asExpr()
+    }
+
+    API::Node getImportNode() { result = imp }
+  }
+
+  /**
+   * A live-server middleware definition.
+   * `live-server` uses the `connect` library under the hood, so the model is based on the `connect` model.
+   */
+  class RouteHandler extends Connect::RouteHandler, DataFlow::FunctionNode {
+    RouteHandler() { this = any(RouteSetup setup).getARouteHandler() }
+
+    override Parameter getRouteHandlerParameter(string kind) {
+      result = ConnectExpressShared::getRouteHandlerParameter(astNode, kind)
+    }
+  }
+
+  /**
+   * The call to `require("live-server").start()`, seen as a route setup.
+   */
+  class RouteSetup extends MethodCallExpr, HTTP::Servers::StandardRouteSetup {
+    ServerDefinition server;
+    API::CallNode call;
+
+    RouteSetup() {
+      call = server.getImportNode().getMember("start").getACall() and
+      this = call.asExpr()
+    }
+
+    override DataFlow::SourceNode getARouteHandler() {
+      exists(DataFlow::SourceNode middleware |
+        middleware = call.getParameter(0).getMember("middleware").getAValueReachingRhs()
+      |
+        result = middleware.getAMemberCall(["push", "unshift"]).getArgument(0).getAFunctionValue()
+        or
+        result = middleware.(DataFlow::ArrayCreationNode).getAnElement().getAFunctionValue()
+      )
+    }
+
+    override Expr getServer() { result = server }
+  }
+}

javascript/ql/lib/semmle/javascript/security/dataflow/MissingRateLimiting.qll

@@ -29,7 +29,7 @@ private import semmle.javascript.frameworks.ConnectExpressShared::ConnectExpress
 /**
  * A route handler that should be rate-limited.
  */
-abstract class ExpensiveRouteHandler extends HTTP::RouteHandler {
+abstract class ExpensiveRouteHandler extends DataFlow::Node {
   Express::RouteHandler impl;
 
   ExpensiveRouteHandler() { this = impl }
@@ -42,10 +42,6 @@ abstract class ExpensiveRouteHandler extends HTTP::RouteHandler {
    * `referenceLabel` are ignored and should be bound to dummy values.
    */
   abstract predicate explain(string explanation, DataFlow::Node reference, string referenceLabel);
-
-  override HTTP::HeaderDefinition getAResponseHeader(string name) {
-    result = impl.getAResponseHeader(name)
-  }
 }
 
 /**