Fixing queries based on suggestions/comments.

TODO: Auto-formatting is still pending (need guidance on how to enable it on my environment). Thanks
This commit is contained in:
Raul Garcia (MSFT)
2020-07-29 17:14:37 -07:00
parent 83e9d052d9
commit 7923c480af
5 changed files with 25 additions and 35 deletions

View File

@@ -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>

View File

@@ -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"

View File

@@ -3,7 +3,7 @@
* @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
*/
@@ -11,16 +11,11 @@
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())
}
}

View File

@@ -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."

View File

@@ -10,9 +10,9 @@ using System.Collections.Generic;
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 @@ namespace DataSetSerializationTest
*/
[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 @@ namespace DataSetSerializationTest
{
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!");
}