Merge pull request #9782 from tamasvajk/cs/newtonsoft-deserialization

C#: Fix unsafe deserialization with `JsonConvert.DeserializeObject`

Author: tamasvajk

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

@@ -889,7 +889,7 @@ private class YamlDotNetDeserializerDeserializeMethodSink extends ConstructorOrS
 }
 
 /** Newtonsoft.Json.JsonConvert */
-private class NewtonsoftJsonConvertDeserializeObjectMethodSink extends ConstructorOrStaticMethodSink {
+private class NewtonsoftJsonConvertDeserializeObjectMethodSink extends Sink {
   NewtonsoftJsonConvertDeserializeObjectMethodSink() {
     exists(MethodCall mc, Method m |
       m = mc.getTarget() and

csharp/ql/src/change-notes/2022-08-11-unsafe-deserialization.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The query `cs/unsafe-deserialization-untrusted-input` is not reporting on all calls of `JsonConvert.DeserializeObject` any longer, it only covers cases that explicitly use unsafe serialization settings.

csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeDeserializationUntrustedInputNewtonsoftJson/Test.cs

@@ -0,0 +1,27 @@
+using Newtonsoft;
+using Newtonsoft.Json;
+using System.Web.UI.WebControls;
+
+class Test
+{
+    public static object Deserialize1(TextBox data)
+    {
+        return JsonConvert.DeserializeObject(data.Text, new JsonSerializerSettings
+        {
+            TypeNameHandling = TypeNameHandling.None // OK
+        });
+    }
+
+    public static object Deserialize2(TextBox data)
+    {
+        return JsonConvert.DeserializeObject(data.Text, new JsonSerializerSettings
+        {
+            TypeNameHandling = TypeNameHandling.Auto // BAD
+        });
+    }
+
+    public static object Deserialize(TextBox data)
+    {
+        return JsonConvert.DeserializeObject(data.Text); // OK, not checking if JsonSerializerSettings is set globally with unsafe settings
+    }
+}

csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeDeserializationUntrustedInputNewtonsoftJson/UnsafeDeserializationUntrustedInput.expected

@@ -0,0 +1,21 @@
+edges
+| ../../../../resources/stubs/Newtonsoft.Json/13.0.1/Newtonsoft.Json.cs:930:20:930:20 | 4 : Int32 | Test.cs:19:32:19:52 | access to constant Auto : Int32 |
+| Test.cs:9:46:9:49 | access to parameter data : TextBox | Test.cs:9:46:9:54 | access to property Text |
+| Test.cs:17:46:17:49 | access to parameter data : TextBox | Test.cs:17:46:17:54 | access to property Text |
+| Test.cs:19:32:19:52 | access to constant Auto : Int32 | Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings |
+| Test.cs:19:32:19:52 | access to constant Auto : TypeNameHandling | Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings |
+| Test.cs:25:46:25:49 | access to parameter data : TextBox | Test.cs:25:46:25:54 | access to property Text |
+nodes
+| ../../../../resources/stubs/Newtonsoft.Json/13.0.1/Newtonsoft.Json.cs:930:20:930:20 | 4 : Int32 | semmle.label | 4 : Int32 |
+| Test.cs:9:46:9:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
+| Test.cs:9:46:9:54 | access to property Text | semmle.label | access to property Text |
+| Test.cs:17:46:17:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
+| Test.cs:17:46:17:54 | access to property Text | semmle.label | access to property Text |
+| Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings | semmle.label | object creation of type JsonSerializerSettings |
+| Test.cs:19:32:19:52 | access to constant Auto : Int32 | semmle.label | access to constant Auto : Int32 |
+| Test.cs:19:32:19:52 | access to constant Auto : TypeNameHandling | semmle.label | access to constant Auto : TypeNameHandling |
+| Test.cs:25:46:25:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
+| Test.cs:25:46:25:54 | access to property Text | semmle.label | access to property Text |
+subpaths
+#select
+| Test.cs:17:46:17:54 | access to property Text | Test.cs:17:46:17:49 | access to parameter data : TextBox | Test.cs:17:46:17:54 | access to property Text | $@ flows to unsafe deserializer. | Test.cs:17:46:17:49 | access to parameter data : TextBox | User-provided data |

csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeDeserializationUntrustedInputNewtonsoftJson/UnsafeDeserializationUntrustedInput.qlref

@@ -0,0 +1 @@
+Security Features/CWE-502/UnsafeDeserializationUntrustedInput.ql

csharp/ql/test/query-tests/Security Features/CWE-502/UnsafeDeserializationUntrustedInputNewtonsoftJson/options

@@ -0,0 +1 @@
+semmle-extractor-options: /nostdlib /noconfig --load-sources-from-project:${testdir}/../../../../resources/stubs/Newtonsoft.Json/13.0.1/Newtonsoft.Json.csproj ${testdir}/../../../../resources/stubs/System.Web.cs