JS: Add ClientSideRequestForgery and split request-forgery results between the two

Author: asgerf

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

@@ -0,0 +1,41 @@
+/**
+ * Provides a taint-tracking configuration for reasoning about request
+ * forgery.
+ *
+ * Note, for performance reasons: only import this file if
+ * `RequestForgery::Configuration` is needed, otherwise
+ * `RequestForgeryCustomizations` should be imported instead.
+ */
+
+import javascript
+import UrlConcatenation
+import RequestForgeryCustomizations::RequestForgery
+
+/**
+ * A taint tracking configuration for client-side request forgery.
+ */
+class Configuration extends TaintTracking::Configuration {
+  Configuration() { this = "ClientSideRequestForgery" }
+
+  override predicate isSource(DataFlow::Node source) {
+    exists(Source src |
+      source = src and
+      not src.isServerSide()
+    )
+  }
+
+  override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  override predicate isSanitizer(DataFlow::Node node) {
+    super.isSanitizer(node) or
+    node instanceof Sanitizer
+  }
+
+  override predicate isSanitizerEdge(DataFlow::Node source, DataFlow::Node sink) {
+    sanitizingPrefixEdge(source, sink)
+  }
+
+  override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
+    isAdditionalRequestForgeryStep(pred, succ)
+  }
+}

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

@@ -9,7 +9,15 @@ module RequestForgery {
   /**
    * A data flow source for request forgery.
    */
-  abstract class Source extends DataFlow::Node { }
+  abstract class Source extends DataFlow::Node {
+    /**
+     * Holds if this source is relevant for server-side request forgery (SSRF).
+     *
+     * Otherwise, it is considered to be a source for client-side request forgery, which is
+     * considered less severe than the server-side variant.
+     */
+    predicate isServerSide() { any() }
+  }
 
   /**
    * A data flow sink for request forgery.
@@ -31,15 +39,18 @@ module RequestForgery {
    */
   abstract class Sanitizer extends DataFlow::Node { }
 
-  /** A source of remote user input, considered as a flow source for request forgery. */
-  private class RemoteFlowSourceAsSource extends Source {
-    RemoteFlowSourceAsSource() {
+  /** A source of server-side remote user input, considered as a flow source for request forgery. */
+  private class ServerSideSource extends Source instanceof RemoteFlowSource {
+    ServerSideSource() { not this instanceof ClientSideRemoteFlowSource }
+  }
+
+  private class ClientSideSource extends Source instanceof ClientSideRemoteFlowSource {
+    ClientSideSource() {
       // Reduce FPs by excluding sources from client-side path or URL
-      exists(RemoteFlowSource src |
-        this = src and
-        not src.(ClientSideRemoteFlowSource).getKind().isPathOrUrl()
-      )
+      not ClientSideRemoteFlowSource.super.getKind().isPathOrUrl()
     }
+
+    override predicate isServerSide() { none() }
   }
 
   /**

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

@@ -17,7 +17,7 @@ import RequestForgeryCustomizations::RequestForgery
 class Configuration extends TaintTracking::Configuration {
   Configuration() { this = "RequestForgery" }
 
-  override predicate isSource(DataFlow::Node source) { source instanceof Source }
+  override predicate isSource(DataFlow::Node source) { source.(Source).isServerSide() }
 
   override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
 

javascript/ql/src/Security/CWE-918/ClientSideRequestForgery.ql

@@ -0,0 +1,23 @@
+/**
+ * @name Client-side request forgery
+ * @description Making a client-to-server request with user-controlled data in the URL allows a request forgery attack
+ *              against the client.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 5.0
+ * @precision medium
+ * @id js/client-side-request-forgery
+ * @tags security
+ *       external/cwe/cwe-918
+ */
+
+import javascript
+import semmle.javascript.security.dataflow.ClientSideRequestForgeryQuery
+import DataFlow::PathGraph
+
+from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink, DataFlow::Node request
+where
+  cfg.hasFlowPath(source, sink) and
+  request = sink.getNode().(Sink).getARequest()
+select request, source, sink, "The $@ of this request depends on $@.", sink.getNode(),
+  sink.getNode().(Sink).getKind(), source, "a user-provided value"