Merge branch 'master' into js/improve-createServer

javascript/ql/src/semmle/javascript/DOM.qll

@@ -253,4 +253,86 @@ module DOM {
       reason = "must not contain any space characters"
     )
   }
+
+  /** Gets a call that queries the DOM for a collection of DOM nodes. */
+  private DataFlow::SourceNode domElementCollection() {
+    exists(string collectionName |
+      collectionName = "getElementsByClassName" or
+      collectionName = "getElementsByName" or
+      collectionName = "getElementsByTagName" or
+      collectionName = "getElementsByTagNameNS" or
+      collectionName = "querySelectorAll"
+    |
+      (
+        result = documentRef().getAMethodCall(collectionName) or
+        result = DataFlow::globalVarRef(collectionName).getACall()
+      )
+    )
+  }
+
+  /** Gets a call that creates a DOM node or queries the DOM for a DOM node. */
+  private DataFlow::SourceNode domElementCreationOrQuery() {
+    exists(string methodName |
+      methodName = "createElement" or
+      methodName = "createElementNS" or
+      methodName = "createRange" or
+      methodName = "getElementById" or
+      methodName = "querySelector"
+    |
+      result = documentRef().getAMethodCall(methodName) or
+      result = DataFlow::globalVarRef(methodName).getACall()
+    )
+  }
+
+  /** Gets a data flow node that refers directly to a value from the DOM. */
+  DataFlow::SourceNode domValueSource() {
+    result.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
+    result = domValueRef().getAPropertyRead() or
+    result = domElementCreationOrQuery() or
+    result = domElementCollection()
+  }
+
+  /** Gets a data flow node that may refer to a value from the DOM. */
+  private DataFlow::SourceNode domValueRef(DataFlow::TypeTracker t) {
+    t.start() and
+    result = domValueSource()
+    or
+    exists(DataFlow::TypeTracker t2 | result = domValueRef(t2).track(t2, t))
+  }
+
+  /** Gets a data flow node that may refer to a value from the DOM. */
+  DataFlow::SourceNode domValueRef() { result = domValueRef(DataFlow::TypeTracker::end()) }
+
+  /** Gets a data flow node that directly refers to a DOM `location` object. */
+  DataFlow::SourceNode locationSource() {
+    result = domValueRef().getAPropertyRead("location")
+    or
+    result = DataFlow::globalVarRef("location")
+  }
+
+  /** Gets a reference to a DOM `location` object. */
+  private DataFlow::SourceNode locationRef(DataFlow::TypeTracker t) {
+    t.start() and
+    result = locationSource()
+    or
+    exists(DataFlow::TypeTracker t2 | result = locationRef(t2).track(t2, t))
+  }
+
+  /** Gets a reference to a DOM `location` object. */
+  DataFlow::SourceNode locationRef() { result = locationRef(DataFlow::TypeTracker::end()) }
+
+  /**
+   * Gets a reference to the `document` object.
+   */
+  private DataFlow::SourceNode documentRef(DataFlow::TypeTracker t) {
+    t.start() and
+    result = DataFlow::globalVarRef("document")
+    or
+    exists(DataFlow::TypeTracker t2 | result = documentRef(t2).track(t2, t))
+  }
+
+  /**
+   * Gets a reference to the 'document' object.
+   */
+  DataFlow::SourceNode documentRef() { result = documentRef(DataFlow::TypeTracker::end()) }
 }

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

@@ -215,6 +215,40 @@ module Firebase {
         result = getArgument(0)
       }
     }
+
+    /**
+     * 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() }
+
+      override DataFlow::SourceNode getARouteHandler() {
+        result = getARouteHandler(DataFlow::TypeBackTracker::end())
+      }
+
+      private DataFlow::SourceNode getARouteHandler(DataFlow::TypeBackTracker t) {
+        t.start() and
+        result = getArgument(0).flow().getALocalSource()
+        or
+        exists(DataFlow::TypeBackTracker t2 | result = getARouteHandler(t2).backtrack(t2, t))
+      }
+
+      override Expr getServer() { none() }
+    }
+
+    /**
+     * A function used as a route handler.
+     */
+    private class RouteHandler extends Express::RouteHandler, HTTP::Servers::StandardRouteHandler,
+    DataFlow::ValueNode {
+      RouteHandler() { this = any(RouteSetup setup).getARouteHandler() }
+
+      override SimpleParameter getRouteHandlerParameter(string kind) {
+        kind = "request" and result = this.(DataFlow::FunctionNode).getParameter(0).getParameter() or
+        kind = "response" and result = this.(DataFlow::FunctionNode).getParameter(1).getParameter()
+      }
+    }
+
   }
 
   /**

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

@@ -1,3 +1,4 @@
+
 /**
  * Provides classes for working with [Koa](https://koajs.com) applications.
  */
@@ -24,7 +25,7 @@ module Koa {
 
     HeaderDefinition() {
       // ctx.set('Cache-Control', 'no-cache');
-      astNode.calls(rh.getAContextExpr(), "set")
+      astNode.calls(rh.getAResponseOrContextExpr(), "set")
       or
       // ctx.response.header('Cache-Control', 'no-cache')
       astNode.calls(rh.getAResponseExpr(), "header")
@@ -58,6 +59,18 @@ module Koa {
      * route handler.
      */
     Expr getAContextExpr() { result.(ContextExpr).getRouteHandler() = this }
+
+    /**
+     * Gets an expression that contains the context or response
+     * object of a route handler invocation.
+     */
+    Expr getAResponseOrContextExpr() { result = getAResponseExpr() or result = getAContextExpr() }
+
+    /**
+     * Gets an expression that contains the context or request
+     * object of a route handler invocation.
+     */
+    Expr getARequestOrContextExpr() { result = getARequestExpr() or result = getAContextExpr() }
   }
 
   /**
@@ -159,35 +172,39 @@ module Koa {
     string kind;
 
     RequestInputAccess() {
-      exists(Expr request | request = rh.getARequestExpr() |
-        // `ctx.request.body`
-        kind = "body" and
-        this.asExpr().(PropAccess).accesses(request, "body")
-        or
-        kind = "parameter" and
-        this = getAQueryParameterAccess(rh)
-        or
+      kind = "parameter" and
+      this = getAQueryParameterAccess(rh)
+      or
+      exists(Expr e | rh.getARequestOrContextExpr() = e |
+        // `ctx.request.url`, `ctx.request.originalUrl`, or `ctx.request.href`
         exists(string propName |
-          // `ctx.request.url`, `ctx.request.originalUrl`, or `ctx.request.href`
           kind = "url" and
-          this.asExpr().(PropAccess).accesses(request, propName)
+          this.asExpr().(PropAccess).accesses(e, propName)
         |
-          propName = "url" or
-          propName = "originalUrl" or
+          propName = "url"
+          or
+          propName = "originalUrl"
+          or
           propName = "href"
         )
-      )
-      or
-      exists(PropAccess cookies |
+        or
+        // `ctx.request.body`
+        e instanceof RequestExpr and
+        kind = "body" and
+        this.asExpr().(PropAccess).accesses(e, "body")
+        or
         // `ctx.cookies.get(<name>)`
-        kind = "cookie" and
-        cookies.accesses(rh.getAContextExpr(), "cookies") and
-        this.asExpr().(MethodCallExpr).calls(cookies, "get")
-      )
-      or
-      exists(RequestHeaderAccess access | access = this |
-        rh = access.getRouteHandler() and
-        kind = "header"
+        exists(PropAccess cookies |
+          e instanceof ContextExpr and
+          kind = "cookie" and
+          cookies.accesses(e, "cookies") and
+          this = cookies.flow().(DataFlow::SourceNode).getAMethodCall("get")
+        )
+        or
+        exists(RequestHeaderAccess access | access = this |
+          rh = access.getRouteHandler() and
+          kind = "header"
+        )
       )
     }
 
@@ -199,8 +216,11 @@ module Koa {
   }
 
   private DataFlow::Node getAQueryParameterAccess(RouteHandler rh) {
-    // `ctx.request.query.name`
-    result.asExpr().(PropAccess).getBase().(PropAccess).accesses(rh.getARequestExpr(), "query")
+    // `ctx.query.name` or `ctx.request.query.name`
+    exists(PropAccess q |
+      q.accesses(rh.getARequestOrContextExpr(), "query") and
+      result = q.flow().(DataFlow::SourceNode).getAPropertyRead()
+    )
   }
 
   /**
@@ -210,18 +230,18 @@ module Koa {
     RouteHandler rh;
 
     RequestHeaderAccess() {
-      exists(Expr request | request = rh.getARequestExpr() |
+      exists(Expr e | e = rh.getARequestOrContextExpr() |
         exists(string propName, PropAccess headers |
           // `ctx.request.header.<name>`, `ctx.request.headers.<name>`
-          headers.accesses(request, propName) and
-          this.asExpr().(PropAccess).accesses(headers, _)
+          headers.accesses(e, propName) and
+          this = headers.flow().(DataFlow::SourceNode).getAPropertyRead()
         |
           propName = "header" or
           propName = "headers"
         )
         or
         // `ctx.request.get(<name>)`
-        this.asExpr().(MethodCallExpr).calls(request, "get")
+        this.asExpr().(MethodCallExpr).calls(e, "get")
       )
     }
 
@@ -264,10 +284,25 @@ module Koa {
 
     ResponseSendArgument() {
       exists(DataFlow::PropWrite pwn |
-        pwn.writes(DataFlow::valueNode(rh.getAResponseExpr()), "body", DataFlow::valueNode(this))
+        pwn
+            .writes(DataFlow::valueNode(rh.getAResponseOrContextExpr()), "body",
+              DataFlow::valueNode(this))
       )
     }
 
     override RouteHandler getRouteHandler() { result = rh }
   }
+
+  /**
+   * An invocation of the `redirect` method of an HTTP response object.
+   */
+  private class RedirectInvocation extends HTTP::RedirectInvocation, MethodCallExpr {
+    RouteHandler rh;
+
+    RedirectInvocation() { this.(MethodCallExpr).calls(rh.getAResponseOrContextExpr(), "redirect") }
+
+    override Expr getUrlArgument() { result = getArgument(0) }
+
+    override RouteHandler getRouteHandler() { result = rh }
+  }
 }

javascript/ql/src/semmle/javascript/security/dataflow/ClientSideUrlRedirect.qll

@@ -121,10 +121,9 @@ module ClientSideUrlRedirect {
       )
       or
       // A call to `location.replace` or `location.assign`
-      exists(MethodCallExpr locationCall, string name |
-        isLocation(locationCall.getReceiver()) and
-        name = locationCall.getMethodName() and
-        astNode = locationCall.getArgument(0)
+      exists(DataFlow::MethodCallNode locationCall, string name |
+        locationCall = DOM::locationRef().getAMethodCall(name) and
+        this = locationCall.getArgument(0)
       |
         name = "replace" or name = "assign"
       )
@@ -134,8 +133,7 @@ module ClientSideUrlRedirect {
       or
       // An assignment to `location.href`, `location.protocol` or `location.hostname`
       exists(DataFlow::PropWrite pw, string propName |
-        isLocation(pw.getBase().asExpr()) and
-        propName = pw.getPropertyName() and
+        pw = DOM::locationRef().getAPropertyWrite(propName) and
         this = pw.getRhs()
       |
         propName = "href" or propName = "protocol" or propName = "hostname"

javascript/ql/src/semmle/javascript/security/dataflow/DOM.qll

@@ -18,73 +18,36 @@ class DOMGlobalVariable extends GlobalVariable {
   }
 }
 
-private DataFlow::SourceNode domElementCollection() {
-  exists(string collectionName |
-    collectionName = "getElementsByClassName" or
-    collectionName = "getElementsByName" or
-    collectionName = "getElementsByTagName" or
-    collectionName = "getElementsByTagNameNS" or
-    collectionName = "querySelectorAll"
-  |
-    (
-      result = document().getAMethodCall(collectionName) or
-      result = DataFlow::globalVarRef(collectionName).getACall()
-    )
-  )
-}
-
-private DataFlow::SourceNode domElementCreationOrQuery() {
-  exists(string methodName |
-    methodName = "createElement" or
-    methodName = "createElementNS" or
-    methodName = "createRange" or
-    methodName = "getElementById" or
-    methodName = "querySelector"
-  |
-    result = document().getAMethodCall(methodName) or
-    result = DataFlow::globalVarRef(methodName).getACall()
-  )
-}
-
-private DataFlow::SourceNode domValueSource() {
-  result.asExpr().(VarAccess).getVariable() instanceof DOMGlobalVariable or
-  result = domValueSource().getAPropertyRead(_) or
-  result = domElementCreationOrQuery() or
-  result = domElementCollection()
-}
-
 /** Holds if `e` could hold a value that comes from the DOM. */
-predicate isDomValue(Expr e) { domValueSource().flowsToExpr(e) }
+predicate isDomValue(Expr e) { DOM::domValueRef().flowsToExpr(e) }
 
 /** Holds if `e` could refer to the `location` property of a DOM node. */
 predicate isLocation(Expr e) {
-  e = domValueSource().getAPropertyReference("location").asExpr() or
+  e = DOM::domValueRef().getAPropertyReference("location").asExpr()
+  or
   e.accessesGlobal("location")
 }
 
 /**
  * Gets a reference to the 'document' object.
  */
-DataFlow::SourceNode document() { result = DataFlow::globalVarRef("document") }
+DataFlow::SourceNode document() { result = DOM::documentRef() }
 
 /** Holds if `e` could refer to the `document` object. */
-predicate isDocument(Expr e) { document().flowsToExpr(e) }
+predicate isDocument(Expr e) { DOM::documentRef().flowsToExpr(e) }
 
 /** Holds if `e` could refer to the document URL. */
 predicate isDocumentURL(Expr e) {
-  exists(Expr base, string propName | e.(PropAccess).accesses(base, propName) |
-    isDocument(base) and
-    (
-      propName = "documentURI" or
-      propName = "documentURIObject" or
-      propName = "location" or
-      propName = "referrer" or
-      propName = "URL"
-    )
-    or
-    isDomValue(base) and propName = "baseUri"
+  exists(string propName | e = DOM::documentRef().getAPropertyRead(propName).asExpr() |
+    propName = "documentURI" or
+    propName = "documentURIObject" or
+    propName = "location" or
+    propName = "referrer" or
+    propName = "URL"
   )
   or
+  e = DOM::domValueRef().getAPropertyRead("baseUri").asExpr()
+  or
   e.accessesGlobal("location")
 }
 
@@ -94,7 +57,7 @@ predicate isDocumentURL(Expr e) {
  * `href`, `hash` and `search`.
  */
 predicate isSafeLocationProperty(PropAccess pacc) {
-  exists(Expr loc, string prop | isLocation(loc) and pacc.accesses(loc, prop) |
+  exists(string prop | pacc = DOM::locationRef().getAPropertyRead(prop).asExpr() |
     prop != "href" and prop != "hash" and prop != "search"
   )
 }

javascript/ql/src/semmle/javascript/security/dataflow/PropertyInjectionShared.qll

@@ -14,7 +14,7 @@ module PropertyInjection {
     node = DataFlow::globalObjectRef()
     or
     // document.write can be accessed
-    isDocument(node.asExpr())
+    node = DOM::documentRef()
     or
     // 'constructor' property leads to the Function constructor.
     node.analyze().getAValue() instanceof AbstractCallable

javascript/ql/src/semmle/javascript/security/dataflow/XpathInjection.qll

@@ -75,7 +75,7 @@ module XpathInjection {
   class DomXpathSink extends Sink {
     DomXpathSink() {
       exists(string m | m = "evaluate" or m = "createExpression" |
-        this = document().getAMethodCall(m).getArgument(0)
+        this = DOM::documentRef().getAMethodCall(m).getArgument(0)
       )
     }
   }