C#: Re-factor some of the data flow configurations used by the UnsafeDeserializationQuery to use the new API.

Author: michaelnebel

csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll

@@ -47,9 +47,11 @@ abstract class Sanitizer extends DataFlow::Node { }
 private class RemoteSource extends Source instanceof RemoteFlowSource { }
 
 /**
+ * DEPRECATED: Use `TaintToObjectMethodTracking` instead.
+ *
  * User input to object method call deserialization flow tracking.
  */
-class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
+deprecated class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
   TaintToObjectMethodTrackingConfig() { this = "TaintToObjectMethodTrackingConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -60,9 +62,27 @@ class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
 }
 
 /**
+ * User input to object method call deserialization flow tracking configuration.
+ */
+private module TaintToObjectMethodTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof InstanceMethodSink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * User input to object method call deserialization flow tracking module.
+ */
+module TaintToObjectMethodTracking = TaintTracking::Global<TaintToObjectMethodTrackingConfig>;
+
+/**
+ * DEPRECATED: Use `JsonConvertTracking` instead.
+ *
  * User input to `JsonConvert` call deserialization flow tracking.
  */
-class JsonConvertTrackingConfig extends TaintTracking::Configuration {
+deprecated class JsonConvertTrackingConfig extends TaintTracking::Configuration {
   JsonConvertTrackingConfig() { this = "JsonConvertTrackingConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -74,6 +94,24 @@ class JsonConvertTrackingConfig extends TaintTracking::Configuration {
   override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
 }
 
+/**
+ * User input to `JsonConvert` call deserialization flow tracking configuration.
+ */
+private module JsonConvertTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) {
+    sink instanceof NewtonsoftJsonConvertDeserializeObjectMethodSink
+  }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * User input to `JsonConvert` call deserialization flow tracking module.
+ */
+module JsonConvertTracking = TaintTracking::Global<JsonConvertTrackingConfig>;
+
 /**
  * DEPRECATED: Use `TypeNameTracking` instead.
  *
@@ -186,9 +224,12 @@ private module TypeNameTrackingConfig implements DataFlow::ConfigSig {
 module TypeNameTracking = DataFlow::Global<TypeNameTrackingConfig>;
 
 /**
+ * DEPRECATED: Use `TaintToConstructorOrStaticMethodTracking` instead.
+ *
  * User input to static method or constructor call deserialization flow tracking.
  */
-class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Configuration {
+deprecated class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Configuration
+{
   TaintToConstructorOrStaticMethodTrackingConfig() {
     this = "TaintToConstructorOrStaticMethodTrackingConfig"
   }
@@ -201,9 +242,28 @@ class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Conf
 }
 
 /**
+ * User input to static method or constructor call deserialization flow tracking configuration.
+ */
+private module TaintToConstructorOrStaticMethodTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof ConstructorOrStaticMethodSink }
+
+  predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
+}
+
+/**
+ * User input to static method or constructor call deserialization flow tracking module.
+ */
+module TaintToConstructorOrStaticMethodTracking =
+  TaintTracking::Global<TaintToConstructorOrStaticMethodTrackingConfig>;
+
+/**
+ * DEPRECATED: Use `TaintToObjectTypeTracking` instead.
+ *
  * User input to instance type flow tracking.
  */
-class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
+deprecated class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
   TaintToObjectTypeTrackingConfig() { this = "TaintToObjectTypeTrackingConfig" }
 
   override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -234,9 +294,47 @@ class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
 }
 
 /**
+ * User input to instance type flow tracking config.
+ */
+private module TaintToObjectTypeTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodCall mc |
+      mc.getTarget() instanceof UnsafeDeserializer and
+      sink.asExpr() = mc.getQualifier()
+    )
+  }
+
+  predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
+    exists(MethodCall mc, Method m |
+      m = mc.getTarget() and
+      m.getDeclaringType().hasQualifiedName("System", "Type") and
+      m.hasName("GetType") and
+      m.isStatic() and
+      n1.asExpr() = mc.getArgument(0) and
+      n2.asExpr() = mc
+    )
+    or
+    exists(ObjectCreation oc |
+      n1.asExpr() = oc.getAnArgument() and
+      n2.asExpr() = oc and
+      oc.getObjectType() instanceof StrongTypeDeserializer
+    )
+  }
+}
+
+/**
+ * User input to instance type flow tracking module.
+ */
+module TaintToObjectTypeTracking = TaintTracking::Global<TaintToObjectTypeTrackingConfig>;
+
+/**
+ * DEPRECATED: Use `WeakTypeCreationToUsageTracking` instead.
+ *
  * Unsafe deserializer creation to usage tracking config.
  */
-class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuration {
+deprecated class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuration {
   WeakTypeCreationToUsageTrackingConfig() { this = "DeserializerCreationToUsageTrackingConfig" }
 
   override predicate isSource(DataFlow::Node source) {
@@ -254,6 +352,31 @@ class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuratio
   }
 }
 
+/**
+ * Unsafe deserializer creation to usage tracking config.
+ */
+private module WeakTypeCreationToUsageTrackingConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) {
+    exists(ObjectCreation oc |
+      oc.getObjectType() instanceof WeakTypeDeserializer and
+      source.asExpr() = oc
+    )
+  }
+
+  predicate isSink(DataFlow::Node sink) {
+    exists(MethodCall mc |
+      mc.getTarget() instanceof UnsafeDeserializer and
+      sink.asExpr() = mc.getQualifier()
+    )
+  }
+}
+
+/**
+ * Unsafe deserializer creation to usage tracking module.
+ */
+module WeakTypeCreationToUsageTracking =
+  TaintTracking::Global<WeakTypeCreationToUsageTrackingConfig>;
+
 /**
  * Safe deserializer creation to usage tracking config.
  */

csharp/ql/src/Security Features/CWE-502/UnsafeDeserializationUntrustedInput.ql

@@ -13,43 +13,40 @@
 
 import csharp
 import semmle.code.csharp.security.dataflow.UnsafeDeserializationQuery
-import DataFlow::PathGraph
+import Flow::PathGraph
 
-from DataFlow::PathNode userInput, DataFlow::PathNode deserializeCallArg
+module Flow =
+  DataFlow::MergePathGraph3<TaintToObjectMethodTracking::PathNode,
+    TaintToConstructorOrStaticMethodTracking::PathNode, JsonConvertTracking::PathNode,
+    TaintToObjectMethodTracking::PathGraph, TaintToConstructorOrStaticMethodTracking::PathGraph,
+    JsonConvertTracking::PathGraph>;
+
+from Flow::PathNode userInput, Flow::PathNode deserializeCallArg
 where
-  exists(TaintToObjectMethodTrackingConfig taintTracking |
-    // all flows from user input to deserialization with weak and strong type serializers
-    taintTracking.hasFlowPath(userInput, deserializeCallArg)
-  ) and
+  // all flows from user input to deserialization with weak and strong type serializers
+  TaintToObjectMethodTracking::flowPath(userInput.asPathNode1(), deserializeCallArg.asPathNode1()) and
   // intersect with strong types, but user controlled or weak types deserialization usages
   (
-    exists(
-      DataFlow::Node weakTypeUsage,
-      WeakTypeCreationToUsageTrackingConfig weakTypeDeserializerTracking, MethodCall mc
-    |
-      weakTypeDeserializerTracking.hasFlowTo(weakTypeUsage) and
+    exists(DataFlow::Node weakTypeUsage, MethodCall mc |
+      WeakTypeCreationToUsageTracking::flowTo(weakTypeUsage) and
       mc.getQualifier() = weakTypeUsage.asExpr() and
       mc.getAnArgument() = deserializeCallArg.getNode().asExpr()
     )
     or
-    exists(
-      TaintToObjectTypeTrackingConfig userControlledTypeTracking, DataFlow::Node taintedTypeUsage,
-      MethodCall mc
-    |
-      userControlledTypeTracking.hasFlowTo(taintedTypeUsage) and
+    exists(DataFlow::Node taintedTypeUsage, MethodCall mc |
+      TaintToObjectTypeTracking::flowTo(taintedTypeUsage) and
       mc.getQualifier() = taintedTypeUsage.asExpr() and
       mc.getAnArgument() = deserializeCallArg.getNode().asExpr()
     )
   )
   or
   // no type check needed - straightforward taint -> sink
-  exists(TaintToConstructorOrStaticMethodTrackingConfig taintTracking2 |
-    taintTracking2.hasFlowPath(userInput, deserializeCallArg)
-  )
+  TaintToConstructorOrStaticMethodTracking::flowPath(userInput.asPathNode2(),
+    deserializeCallArg.asPathNode2())
   or
   // JsonConvert static method call, but with additional unsafe typename tracking
-  exists(JsonConvertTrackingConfig taintTrackingJsonConvert, DataFlow::Node settingsCallArg |
-    taintTrackingJsonConvert.hasFlowPath(userInput, deserializeCallArg) and
+  exists(DataFlow::Node settingsCallArg |
+    JsonConvertTracking::flowPath(userInput.asPathNode3(), deserializeCallArg.asPathNode3()) and
     TypeNameTracking::flow(_, settingsCallArg) and
     deserializeCallArg.getNode().asExpr().getParent() = settingsCallArg.asExpr().getParent()
   )