Python: Move FullServerSideRequestForgery and PartialServerSideRequestForgery to new dataflow API

Author: RasmusWL

python/ql/lib/semmle/python/security/dataflow/ServerSideRequestForgeryQuery.qll

@@ -13,6 +13,8 @@ import semmle.python.Concepts
 import ServerSideRequestForgeryCustomizations::ServerSideRequestForgery
 
 /**
+ * DEPRECATED: Use `FullServerSideRequestForgeryFlow` module instead.
+ *
  * A taint-tracking configuration for detecting "Server-side request forgery" vulnerabilities.
  *
  * This configuration has a sanitizer to limit results to cases where attacker has full control of URL.
@@ -21,7 +23,7 @@ import ServerSideRequestForgeryCustomizations::ServerSideRequestForgery
  * You should use the `fullyControlledRequest` to only select results where all
  * URL parts are fully controlled.
  */
-class FullServerSideRequestForgeryConfiguration extends TaintTracking::Configuration {
+deprecated class FullServerSideRequestForgeryConfiguration extends TaintTracking::Configuration {
   FullServerSideRequestForgeryConfiguration() { this = "FullServerSideRequestForgery" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -39,24 +41,51 @@ class FullServerSideRequestForgeryConfiguration extends TaintTracking::Configura
   }
 }
 
+/**
+ * This configuration has a sanitizer to limit results to cases where attacker has full control of URL.
+ * See `PartialServerSideRequestForgery` for a variant without this requirement.
+ *
+ * You should use the `fullyControlledRequest` to only select results where all
+ * URL parts are fully controlled.
+ */
+private module FullServerSideRequestForgeryConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) {
+    node instanceof Sanitizer
+    or
+    node instanceof FullUrlControlSanitizer
+  }
+}
+
+/**
+ * Global taint-tracking for detecting "Full server-side request forgery" vulnerabilities.
+ *
+ * You should use the `fullyControlledRequest` to only select results where all
+ * URL parts are fully controlled.
+ */
+module FullServerSideRequestForgeryFlow = TaintTracking::Global<FullServerSideRequestForgeryConfig>;
+
 /**
  * Holds if all URL parts of `request` is fully user controlled.
  */
 predicate fullyControlledRequest(Http::Client::Request request) {
-  exists(FullServerSideRequestForgeryConfiguration fullConfig |
-    forall(DataFlow::Node urlPart | urlPart = request.getAUrlPart() |
-      fullConfig.hasFlow(_, urlPart)
-    )
+  forall(DataFlow::Node urlPart | urlPart = request.getAUrlPart() |
+    FullServerSideRequestForgeryFlow::flow(_, urlPart)
   )
 }
 
 /**
+ * DEPRECATED: Use `FullServerSideRequestForgeryFlow` module instead.
+ *
  * A taint-tracking configuration for detecting "Server-side request forgery" vulnerabilities.
  *
  * This configuration has results, even when the attacker does not have full control over the URL.
  * See `FullServerSideRequestForgeryConfiguration`, and the `fullyControlledRequest` predicate.
  */
-class PartialServerSideRequestForgeryConfiguration extends TaintTracking::Configuration {
+deprecated class PartialServerSideRequestForgeryConfiguration extends TaintTracking::Configuration {
   PartialServerSideRequestForgeryConfiguration() { this = "PartialServerSideRequestForgery" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -69,3 +98,21 @@ class PartialServerSideRequestForgeryConfiguration extends TaintTracking::Config
     guard instanceof SanitizerGuard
   }
 }
+
+/**
+ * This configuration has results, even when the attacker does not have full control over the URL.
+ * See `FullServerSideRequestForgeryConfiguration`, and the `fullyControlledRequest` predicate.
+ */
+private module PartialServerSideRequestForgeryConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * Global taint-tracking for detecting "partial server-side request forgery" vulnerabilities.
+ */
+module PartialServerSideRequestForgeryFlow =
+  TaintTracking::Global<PartialServerSideRequestForgeryConfig>;

python/ql/src/Security/CWE-918/FullServerSideRequestForgery.ql

@@ -12,14 +12,14 @@
 
 import python
 import semmle.python.security.dataflow.ServerSideRequestForgeryQuery
-import DataFlow::PathGraph
+import FullServerSideRequestForgeryFlow::PathGraph
 
 from
-  FullServerSideRequestForgeryConfiguration fullConfig, DataFlow::PathNode source,
-  DataFlow::PathNode sink, Http::Client::Request request
+  FullServerSideRequestForgeryFlow::PathNode source,
+  FullServerSideRequestForgeryFlow::PathNode sink, Http::Client::Request request
 where
   request = sink.getNode().(Sink).getRequest() and
-  fullConfig.hasFlowPath(source, sink) and
+  FullServerSideRequestForgeryFlow::flowPath(source, sink) and
   fullyControlledRequest(request)
 select request, source, sink, "The full URL of this request depends on a $@.", source.getNode(),
   "user-provided value"

python/ql/src/Security/CWE-918/PartialServerSideRequestForgery.ql

@@ -12,14 +12,14 @@
 
 import python
 import semmle.python.security.dataflow.ServerSideRequestForgeryQuery
-import DataFlow::PathGraph
+import PartialServerSideRequestForgeryFlow::PathGraph
 
 from
-  PartialServerSideRequestForgeryConfiguration partialConfig, DataFlow::PathNode source,
-  DataFlow::PathNode sink, Http::Client::Request request
+  PartialServerSideRequestForgeryFlow::PathNode source,
+  PartialServerSideRequestForgeryFlow::PathNode sink, Http::Client::Request request
 where
   request = sink.getNode().(Sink).getRequest() and
-  partialConfig.hasFlowPath(source, sink) and
+  PartialServerSideRequestForgeryFlow::flowPath(source, sink) and
   not fullyControlledRequest(request)
 select request, source, sink, "Part of the URL of this request depends on a $@.", source.getNode(),
   "user-provided value"