Fixing queries based on suggestions/comments.

TODO: Auto-formatting is still pending (need guidance on how to enable it on my environment). Thanks

Author: raulgarciamsft

csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qhelp

@@ -12,7 +12,7 @@
 </overview>
 <recommendation>
 
-<p>Please review the <a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a> before makign use of these types for serialization.</p>
+<p>Please review the <a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a> before making use of these types for serialization.</p>
 
 </recommendation>
 <references>

csharp/ql/src/experimental/Security Features/Serialization/DataSetSerialization.qll

@@ -17,10 +17,8 @@ abstract class DataSetOrTableRelatedClass extends Class {
  */
 class DataSetOrTable extends DataSetOrTableRelatedClass {
   DataSetOrTable() {
-    this.getABaseType*().getQualifiedName().matches("System.Data.DataTable") or
-    this.getABaseType*().getQualifiedName().matches("System.Data.DataSet") or
-    this.getQualifiedName().matches("System.Data.DataTable") or
-    this.getQualifiedName().matches("System.Data.DataSet")
+    this.getABaseType*().getQualifiedName() = "System.Data.DataTable" or
+    this.getABaseType*().getQualifiedName() = "System.Data.DataSet"
   }
 }
 
@@ -32,11 +30,8 @@ class ClassWithDataSetOrTableMember extends DataSetOrTableRelatedClass {
     exists( Property p |
       p = this.getAProperty() |
       p.getType() instanceof DataSetOrTable
-    ) or exists ( AssignableMember am |
-      am = this.getAField() or
-      am = this.getAMember() |
-      am.getType() instanceof DataSetOrTable
-    ) or exists( Property p |
+    ) or this.getAMember().(AssignableMember).getType() instanceof DataSetOrTable
+    or exists( Property p |
       p = this.getAProperty() |
       p.getType() instanceof DataSetOrTable or
       p.getType().(ConstructedGeneric).getATypeArgument() instanceof DataSetOrTable 
@@ -50,12 +45,12 @@ class ClassWithDataSetOrTableMember extends DataSetOrTableRelatedClass {
 class SerializableClass extends Class {
   SerializableClass() {
     (
-      this.getABaseType*().getQualifiedName().matches("System.Xml.Serialization.XmlSerializer") or
-      this.getABaseInterface*().getQualifiedName().matches("System.Runtime.Serialization.ISerializable") or
-      this.getABaseType*().getQualifiedName().matches("System.Runtime.Serialization.XmlObjectSerializer") or
-      this.getABaseInterface*().getQualifiedName().matches("System.Runtime.Serialization.ISerializationSurrogateProvider") or
-      this.getABaseType*().getQualifiedName().matches("System.Runtime.Serialization.XmlSerializableServices") or
-      this.getABaseInterface*().getQualifiedName().matches("System.Xml.Serialization.IXmlSerializable")
+      this.getABaseType*().getQualifiedName() = "System.Xml.Serialization.XmlSerializer" or
+      this.getABaseInterface*().getQualifiedName() = "System.Runtime.Serialization.ISerializable" or
+      this.getABaseType*().getQualifiedName() = "System.Runtime.Serialization.XmlObjectSerializer" or
+      this.getABaseInterface*().getQualifiedName() = "System.Runtime.Serialization.ISerializationSurrogateProvider" or
+      this.getABaseType*().getQualifiedName() = "System.Runtime.Serialization.XmlSerializableServices" or
+      this.getABaseInterface*().getQualifiedName() = "System.Xml.Serialization.IXmlSerializable"
     ) or exists( Attribute a |
       a = this.getAnAttribute() |
       a.getType().getQualifiedName().toString() = "System.SerializableAttribute"

csharp/ql/src/experimental/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql

@@ -3,24 +3,19 @@
  * @description Unsafe type is used in data contract serializer. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details."
  * @kind problem
  * @problem.severity error
- * @precision high
+ * @precision medium
  * @id cs/dataset-serialization/unsafe-type-used-data-contract-serializer
  * @tags security
  */
 
 import csharp
 import DataSetSerialization
 
-predicate isClassDependingOnDataSetOrTable( Class c ) {
-  c instanceof DataSetOrTableRelatedClass
-}
-
-predicate xmlSerializerConstructorTypeParameter (Expr e) {
+predicate xmlSerializerConstructorArgument (Expr e) {
   exists (ObjectCreation oc, Constructor c |
     e = oc.getArgument(0) |
     c = oc.getTarget() and
     ( 
-      c.getDeclaringType().hasQualifiedName("System.Xml.Serialization.XmlSerializer") or
       c.getDeclaringType().getABaseType*().hasQualifiedName("System.Xml.Serialization.XmlSerializer")
     )
   )
@@ -30,9 +25,9 @@ predicate unsafeDataContractTypeCreation (Expr e) {
   exists(MethodCall gt | 
     gt.getTarget().getName() = "GetType" and
     e = gt and
-    isClassDependingOnDataSetOrTable(gt.getQualifier().getType())
+    gt.getQualifier().getType() instanceof DataSetOrTableRelatedClass
   ) or
-  isClassDependingOnDataSetOrTable(e.(TypeofExpr).getTypeAccess().getTarget())
+  e.(TypeofExpr).getTypeAccess().getTarget() instanceof DataSetOrTableRelatedClass
 }
 
 class Conf extends DataFlow::Configuration {
@@ -45,7 +40,7 @@ class Conf extends DataFlow::Configuration {
     }
 
   override predicate isSink(DataFlow::Node node) {
-      xmlSerializerConstructorTypeParameter (node.asExpr())
+      xmlSerializerConstructorArgument (node.asExpr())
     }
 }
 

csharp/ql/src/experimental/Security Features/Serialization/XmlDeserializationWithDataSet.ql

@@ -11,6 +11,6 @@
 import csharp
 import DataSetSerialization
 
-from UnsafeXmlReadMethodCall mc, Method m
-where m.getACall() = mc
+from UnsafeXmlReadMethodCall mc
+where exists( Method m | m.getACall() = mc )
 select mc, "Making an XML deserialization call with a type derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details."

csharp/ql/test/experimental/Security Features/Serialization/test0.cs

@@ -10,9 +10,9 @@
 
 namespace DataSetSerializationTest
 {
-    public class DerivesFromDeprecatedType1 : XmlSerializer // bug
+    public class DerivesFromDeprecatedType1 : XmlSerializer // warning:DefiningDatasetRelatedType.ql
     {
-        public DataSet MyDataSet { get; set; }  // bug
+        public DataSet MyDataSet { get; set; }  // bug:DefiningPotentiallyUnsafeXmlSerializer.ql
 
         public DerivesFromDeprecatedType1()
         {
@@ -56,9 +56,9 @@ public override void WriteEndObject(XmlDictionaryWriter writer) { }
     */
 
     [Serializable()]
-    public class AttributeSerializer01  // bug
+    public class AttributeSerializer01  // warning:DefiningDatasetRelatedType.ql
     {
-        private DataSet MyDataSet;  // bug
+        private DataSet MyDataSet;  // bug:DefiningPotentiallyUnsafeXmlSerializer.ql
 
         AttributeSerializer01()
         {
@@ -85,15 +85,15 @@ static void datatable_readxmlschema_01(string fileName)
             {
                 DataTable newTable = new DataTable();
                 System.Xml.XmlTextReader reader = new System.Xml.XmlTextReader(fs);
-                newTable.ReadXmlSchema(reader); //bug
+                newTable.ReadXmlSchema(reader); //bug:XmlDeserializationWithDataSet.ql
             }
         }
 
         static void Main(string[] args)
         {
 
-            XmlSerializer x = new XmlSerializer(typeof(DataSet));   // bug
-            XmlSerializer y = new XmlSerializer(typeof(AttributeSerializer01)); //bug
+            XmlSerializer x = new XmlSerializer(typeof(DataSet));   // bug:UnsafeTypeUsedDataContractSerializer.ql
+            XmlSerializer y = new XmlSerializer(typeof(AttributeSerializer01)); //bug:UnsafeTypeUsedDataContractSerializer.ql
 
             Console.WriteLine("Hello World!");
         }