C#: Exclude fields that are created

Author: calumgrant

csharp/ql/src/Security Features/CWE-020/RuntimeChecksBypass.ql

@@ -14,6 +14,7 @@
 
 import semmle.code.csharp.serialization.Serialization
 import semmle.code.csharp.controlflow.Guards
+import semmle.code.csharp.dataflow.DataFlow
 
 /**
  * The result is a write to the field `f`, assigning it the value
@@ -32,7 +33,9 @@ GuardedExpr checkedWrite(Field f, Variable v, IfStmt check) {
 Expr uncheckedWrite(Callable callable, Field f) {
   result = f.getAnAssignedValue() and
   result.getEnclosingCallable() = callable and
-  not callable.calls*(checkedWrite(f, _, _).getEnclosingCallable())
+  not callable.calls*(checkedWrite(f, _, _).getEnclosingCallable()) and
+  // Exclude object creations because they were not deserialized
+  not exists(ObjectCreation src | DataFlow::localExprFlow(src, result))
 }
 
 from BinarySerializableType t, Field f, IfStmt check, Expr write, Expr unsafeWrite

csharp/ql/test/query-tests/Security Features/CWE-020/RuntimeChecksBypass.cs

@@ -10,14 +10,14 @@ public Test1(string v)
     {
         if (v == "valid")
         {
-            f = v /* safe write */;
+            f = v;  // GOOD
         }
     }
 
     [OnDeserializing]
     public void Deserialize()
     {
-        f = "invalid" /* unsafe write */;
+        f = "invalid";  // BAD
     }
 }
 
@@ -30,19 +30,19 @@ public Test2(string v)
     {
         if (v == "valid")
         {
-            f = v /* safe write */;
+            f = v;  // GOOD
         }
     }
 
     [OnDeserializing]
     public void Deserialize()
     {
         var v = "invalid";
-        f = v /* unsafe write -- false negative */;
+        f = v;  // BAD: False negative
 
         if (v == "valid")
         {
-            f = v;  /* safe write */
+            f = v;  // GOOD
         }
     }
 }
@@ -56,25 +56,25 @@ public Test3(string v)
     {
         if (v == "valid")
         {
-            f = v /* safe write */;
+            f = v;  // GOOD
         }
     }
 
     [OnDeserializing]
     public void Deserialize()
     {
         var v = "invalid";
-        f = v /* unsafe write -- false negative */;
+        f = v;  // GOOD: False negative
         Assign(v);
     }
 
     private void Assign(string v)
     {
-        f = v /* unsafe write -- false negative */;
+        f = v;  // GOOD: False negative
 
         if (v == "valid")
         {
-            f = v /* safe write */;
+            f = v;  // GOOD
         }
     }
 }
@@ -88,7 +88,7 @@ public Test4(string v)
     {
         if (v == "valid")
         {
-            f = v /* safe write */;
+            f = v;  // GOOD
         }
     }
 
@@ -102,7 +102,7 @@ public void Deserialize()
 
     private void Assign(string v)
     {
-        f = v /* safe write */;
+        f = v;  // GOOD
     }
 }
 
@@ -115,13 +115,13 @@ public Test5(int age)
     {
         if (age < 0)
             throw new ArgumentException(nameof(age));
-        Age = age /* safe write */;
+        Age = age;  // GOOD
     }
 
     [OnDeserializing]
     void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
     {
-        Age = info.GetInt32("age"); /* unsafe write */;
+        Age = info.GetInt32("age");  // BAD
     }
 }
 
@@ -134,7 +134,7 @@ public Test6(int age)
     {
         if (age < 0)
             throw new ArgumentException(nameof(age));
-        Age = age /* safe write */;
+        Age = age;  // GOOD
     }
 
     [OnDeserializing]
@@ -143,7 +143,7 @@ void ISerializable.GetObjectData(SerializationInfo info, StreamingContext contex
         int age = info.GetInt32("age");
         if (age < 0)
             throw new SerializationException("age");
-        Age = age; /* safe write */;
+        Age = age;  // GOOD
     }
 }
 
@@ -156,7 +156,7 @@ public Test7(int age)
     {
         if (age < 0)
             throw new ArgumentException(nameof(age));
-        Age = age /* safe write */;
+        Age = age;  // GOOD
     }
 
     [OnDeserializing]
@@ -165,6 +165,27 @@ void ISerializable.GetObjectData(SerializationInfo info, StreamingContext contex
         int age = info.GetInt32("age");
         if (false)
             throw new SerializationException("age");
-        Age = age; /* unsafe write */;
+        Age = age;  // BAD
+    }
+}
+
+[Serializable]
+public class Test8 : ISerializable
+{
+    string Options;
+
+    public int Age;
+
+    public Test8(string options)
+    {
+        if (options == null)
+            throw new ArgumentNullException(nameof(options));
+        Options = options;  // GOOD
+    }
+
+    [OnDeserializing]
+    void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)
+    {
+        Options = new string("");  // GOOD: A created object
     }
 }