JsonConvert

Author: edvraa

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

@@ -46,5 +46,16 @@ where
   exists(TaintToConstructorOrStaticMethodTrackingConfig taintTracking2 |
     taintTracking2.hasFlowPath(userInput, deserializeCallArg)
   )
+  or
+  // JsonConvert static method call, but with additional unsafe typename tracking
+  exists(
+    JsonConvertTrackingConfig taintTrackingJsonConvert, TypeNameTrackingConfig typenameTracking,
+    DataFlow::PathNode settingsCallArg
+  |
+    taintTrackingJsonConvert.hasFlowPath(userInput, deserializeCallArg) and
+    typenameTracking.hasFlowPath(_, settingsCallArg) and
+    deserializeCallArg.getNode().asExpr().getParent() =
+      settingsCallArg.getNode().asExpr().getParent()
+  )
 select deserializeCallArg, userInput, deserializeCallArg, "$@ flows to unsafe deserializer.",
   userInput, "User-provided data"

csharp/ql/src/semmle/code/csharp/frameworks/JsonNET.qll

@@ -17,6 +17,19 @@ module JsonNET {
     JsonClass() { this.getParent() instanceof JsonNETNamespace }
   }
 
+  /** Newtonsoft.Json.TypeNameHandling enum */
+  class TypeNameHandlingEnum extends Enum {
+    TypeNameHandlingEnum() {
+      this.getParent() instanceof JsonNETNamespace and
+      this.hasName("TypeNameHandling")
+    }
+  }
+
+  /** Newtonsoft.Json.JsonSerializerSettings class */
+  class JsonSerializerSettingsClass extends JsonClass {
+    JsonSerializerSettingsClass() { this.hasName("JsonSerializerSettings") }
+  }
+
   /** The class `Newtonsoft.Json.JsonConvert`. */
   class JsonConvertClass extends JsonClass, LibraryTypeDataFlow {
     JsonConvertClass() { this.hasName("JsonConvert") }

csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll

@@ -59,7 +59,7 @@ module UnsafeDeserialization {
    * User input to object method call deserialization flow tracking.
    */
   class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
-    TaintToObjectMethodTrackingConfig() { this = "UnsafeDeserialization1" }
+    TaintToObjectMethodTrackingConfig() { this = "TaintToObjectMethodTrackingConfig" }
 
     override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
@@ -68,11 +68,81 @@ module UnsafeDeserialization {
     override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
   }
 
+  /**
+   * User input to `JsonConvert` call deserialization flow tracking.
+   */
+  class JsonConvertTrackingConfig extends TaintTracking::Configuration {
+    JsonConvertTrackingConfig() { this = "JsonConvertTrackingConfig" }
+
+    override predicate isSource(DataFlow::Node source) { source instanceof Source }
+
+    override predicate isSink(DataFlow::Node sink) {
+      sink instanceof NewtonsoftJsonConvertDeserializeObjectMethodSink
+    }
+
+    override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
+  }
+
+  /**
+   * Tracks unsafe `TypeNameHandling` setting to `JsonConvert` call
+   */
+  class TypeNameTrackingConfig extends DataFlow::Configuration {
+    TypeNameTrackingConfig() { this = "TypeNameTrackingConfig" }
+
+    override predicate isSource(DataFlow::Node source) {
+      (
+        source.asExpr() instanceof MemberConstantAccess and
+        source.getType() instanceof TypeNameHandlingEnum
+        or
+        source.asExpr() instanceof IntegerLiteral
+      ) and
+      source.asExpr().hasValue() and
+      not source.asExpr().getValue() = "0"
+    }
+
+    override predicate isSink(DataFlow::Node sink) {
+      exists(MethodCall mc, Method m, Expr expr |
+        m = mc.getTarget() and
+        (
+          not mc.getArgument(0).hasValue() and
+          m instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod
+        ) and
+        expr = mc.getAnArgument() and
+        sink.asExpr() = expr and
+        expr.getType() instanceof JsonSerializerSettingsClass
+      )
+    }
+
+    override predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+      node1.asExpr() instanceof IntegerLiteral and
+      node2.asExpr().(CastExpr).getExpr() = node1.asExpr()
+      or
+      node1.getType() instanceof TypeNameHandlingEnum and
+      exists(PropertyWrite pw, Property p, Assignment a |
+        a.getLValue() = pw and
+        pw.getProperty() = p and
+        p.getDeclaringType() instanceof JsonSerializerSettingsClass and
+        p.hasName("TypeNameHandling") and
+        (
+          node1.asExpr() = a.getRValue() and
+          node2.asExpr() = pw.getQualifier()
+          or
+          exists(ObjectInitializer oi |
+            node1.asExpr() = oi.getAMemberInitializer().getRValue() and
+            node2.asExpr() = oi
+          )
+        )
+      )
+    }
+  }
+
   /**
    * User input to static method or constructor call deserialization flow tracking.
    */
   class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Configuration {
-    TaintToConstructorOrStaticMethodTrackingConfig() { this = "UnsafeDeserialization2" }
+    TaintToConstructorOrStaticMethodTrackingConfig() {
+      this = "TaintToConstructorOrStaticMethodTrackingConfig"
+    }
 
     override predicate isSource(DataFlow::Node source) { source instanceof Source }
 
@@ -826,4 +896,18 @@ module UnsafeDeserialization {
       )
     }
   }
+
+  /** Newtonsoft.Json.JsonConvert */
+  private class NewtonsoftJsonConvertDeserializeObjectMethodSink extends ConstructorOrStaticMethodSink {
+    NewtonsoftJsonConvertDeserializeObjectMethodSink() {
+      exists(MethodCall mc, Method m |
+        m = mc.getTarget() and
+        (
+          not mc.getArgument(0).hasValue() and
+          m instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod
+        ) and
+        this.asExpr() = mc.getArgument(0)
+      )
+    }
+  }
 }

csharp/ql/src/semmle/code/csharp/serialization/Deserializers.qll

@@ -4,6 +4,7 @@
  */
 
 import csharp
+import semmle.code.csharp.frameworks.JsonNET::JsonNET
 
 /** An unsafe deserializer. */
 abstract class UnsafeDeserializer extends Callable { }
@@ -67,6 +68,8 @@ class WeakTypeDeserializer extends Class {
     this instanceof SharpSerializerClass
     or
     this instanceof YamlDotNetDeserializerClass
+    or
+    this instanceof JsonConvertClass
   }
 }
 
@@ -657,3 +660,12 @@ class YamlDotNetDeserializerClasseserializeMethod extends Method, UnsafeDeserial
     )
   }
 }
+
+/** `Newtonsoft.Json.JsonConvert.DeserializeObject` method */
+class NewtonsoftJsonConvertClassDeserializeObjectMethod extends Method, UnsafeDeserializer {
+  NewtonsoftJsonConvertClassDeserializeObjectMethod() {
+    this.getDeclaringType() instanceof JsonConvertClass and
+    this.hasName("DeserializeObject") and
+    this.isStatic()
+  }
+}