Skip to content

Commit c20d763

Browse files
hvitvedhvitved
authored
Merge pull request github#3951 from raulgarciamsft/users/raulgarciamsft/dataset_serialization
C#: DataSet serialization
2 parents 77db87e + 3682a90 commit c20d763

19 files changed

+350
-0
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>The <code>DataSet</code> and <code>DataTable</code> types are legacy .NET components that you can use to represent data sets as managed objects.</p>
7+
8+
<p>While <code>DataSet</code> and <code>DataTable</code> do impose default limitations on the types that are allowed to be present while deserializing XML payloads, <code>DataSet</code> and <code>DataTable</code> are in general not safe when populated with untrusted input.</p>
9+
10+
<p>Please visit <a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a> for more details.</p>
11+
12+
</overview>
13+
<recommendation>
14+
15+
<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>
16+
17+
</recommendation>
18+
<references>
19+
20+
<li>Microsoft Docs<a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a>.</li>
21+
22+
</references>
23+
</qhelp>

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

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
/**
2+
* Provides classes for `DataSet` or `DataTable` deserialization queries.
3+
*
4+
* Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details.
5+
*/
6+
7+
import csharp
8+
9+
/**
10+
* Abstract class that depends or inherits from `DataSet` or `DataTable` types.
11+
*/
12+
abstract class DataSetOrTableRelatedClass extends Class { }
13+
14+
/**
15+
* `DataSet`, `DataTable` types, or any types derived from them.
16+
*/
17+
class DataSetOrTable extends DataSetOrTableRelatedClass {
18+
DataSetOrTable() {
19+
this.getABaseType*().getQualifiedName() = "System.Data.DataTable" or
20+
this.getABaseType*().getQualifiedName() = "System.Data.DataSet"
21+
}
22+
}
23+
24+
/**
25+
* A Class that include a property or generic collection of type `DataSet` and `DataTable`
26+
*/
27+
class ClassWithDataSetOrTableMember extends DataSetOrTableRelatedClass {
28+
ClassWithDataSetOrTableMember() {
29+
this.getAMember().(AssignableMember).getType() instanceof DataSetOrTable
30+
or
31+
exists(Property p | p = this.getAProperty() |
32+
p.getType() instanceof DataSetOrTable or
33+
p.getType().(ConstructedGeneric).getATypeArgument() instanceof DataSetOrTable
34+
)
35+
}
36+
}
37+
38+
/**
39+
* Serializable types
40+
*/
41+
class SerializableClass extends Class {
42+
SerializableClass() {
43+
(
44+
this.getABaseType*().getQualifiedName() = "System.Xml.Serialization.XmlSerializer" or
45+
this.getABaseType*().getQualifiedName() = "System.Runtime.Serialization.ISerializable" or
46+
this.getABaseType*().getQualifiedName() = "System.Runtime.Serialization.XmlObjectSerializer" or
47+
this.getABaseType*().getQualifiedName() =
48+
"System.Runtime.Serialization.ISerializationSurrogateProvider" or
49+
this.getABaseType*().getQualifiedName() =
50+
"System.Runtime.Serialization.XmlSerializableServices" or
51+
this.getABaseType*().getQualifiedName() = "System.Xml.Serialization.IXmlSerializable"
52+
)
53+
or
54+
exists(Attribute a | a = this.getAnAttribute() |
55+
a.getType().getQualifiedName() = "System.SerializableAttribute"
56+
)
57+
}
58+
}
59+
60+
/**
61+
* Holds if the serializable class `c` has a property or field `m` that is of `DataSet` or `DataTable` related type
62+
*/
63+
predicate isClassUnsafeXmlSerializerImplementation(SerializableClass c, AssignableMember am) {
64+
am = c.getAMember() and
65+
am.getType() instanceof DataSetOrTableRelatedClass
66+
}
67+
68+
/**
69+
* Serializable class that has a property or field that is of `DataSet` or `DataTable` related type
70+
*/
71+
class UnsafeXmlSerializerImplementation extends SerializableClass {
72+
UnsafeXmlSerializerImplementation() { isClassUnsafeXmlSerializerImplementation(this, _) }
73+
}
74+
75+
/**
76+
* Method that may be unsafe when used to deserialize DataSet and DataTable related types
77+
*/
78+
class UnsafeXmlReadMethod extends Method {
79+
UnsafeXmlReadMethod() {
80+
this.getQualifiedName() = "System.Data.DataTable.ReadXml"
81+
or
82+
this.getQualifiedName() = "System.Data.DataTable.ReadXmlSchema"
83+
or
84+
this.getQualifiedName() = "System.Data.DataSet.ReadXml"
85+
or
86+
this.getQualifiedName() = "System.Data.DataSet.ReadXmlSchema"
87+
or
88+
this.getName().matches("ReadXml%") and
89+
exists(Class c | c.getAMethod() = this |
90+
c.getABaseType*() instanceof DataSetOrTableRelatedClass
91+
)
92+
}
93+
}
94+
95+
/**
96+
* MethodCall that may be unsafe when used to deserialize DataSet and DataTable related types
97+
*/
98+
class UnsafeXmlReadMethodCall extends MethodCall {
99+
UnsafeXmlReadMethodCall() { exists(UnsafeXmlReadMethod uxrm | uxrm.getACall() = this) }
100+
}

‎csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.qhelp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="DataSetSerialization.qhelp" /></qhelp>

‎csharp/ql/src/experimental/Security Features/Serialization/DefiningDatasetRelatedType.ql

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* @name Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types
3+
* @description Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types may lead to the usage of dangerous functionality. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details.
4+
* @kind problem
5+
* @problem.severity warning
6+
* @id cs/dataset-serialization/defining-dataset-related-type
7+
* @tags security
8+
*/
9+
10+
import csharp
11+
import DataSetSerialization
12+
13+
from DataSetOrTableRelatedClass dstc
14+
where dstc.fromSource()
15+
select dstc,
16+
"Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details."

‎csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerialization.qhelp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="DataSetSerialization.qhelp" /></qhelp>

‎csharp/ql/src/experimental/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
/**
2+
* @name Defining a potentially unsafe XML serializer
3+
* @description Defining an XML serializable class that includes members that derive from DataSet or DataTable type may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details.
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision medium
7+
* @id cs/dataset-serialization/defining-potentially-unsafe-xml-serializer
8+
* @tags security
9+
*/
10+
11+
import csharp
12+
import DataSetSerialization
13+
14+
from UnsafeXmlSerializerImplementation c, Member m
15+
where
16+
c.fromSource() and
17+
isClassUnsafeXmlSerializerImplementation(c, m)
18+
select m,
19+
"Defining an serializable class $@ that has member $@ of a type that is derived from DataSet or DataTable types and may lead to a security problem. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details.",
20+
c, c.toString(), m, m.toString()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="DataSetSerialization.qhelp" /></qhelp>

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

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
/**
2+
* @name Unsafe type is used in data contract serializer
3+
* @description Unsafe type is used in data contract serializer. Please visit https://go.microsoft.com/fwlink/?linkid=2132227 for details."
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision medium
7+
* @id cs/dataset-serialization/unsafe-type-used-data-contract-serializer
8+
* @tags security
9+
*/
10+
11+
import csharp
12+
import DataSetSerialization
13+
14+
predicate xmlSerializerConstructorArgument(Expr e) {
15+
exists(ObjectCreation oc, Constructor c | e = oc.getArgument(0) |
16+
c = oc.getTarget() and
17+
c.getDeclaringType().getABaseType*().hasQualifiedName("System.Xml.Serialization.XmlSerializer")
18+
)
19+
}
20+
21+
predicate unsafeDataContractTypeCreation(Expr e) {
22+
exists(MethodCall gt |
23+
gt.getTarget().getName() = "GetType" and
24+
e = gt and
25+
gt.getQualifier().getType() instanceof DataSetOrTableRelatedClass
26+
)
27+
or
28+
e.(TypeofExpr).getTypeAccess().getTarget() instanceof DataSetOrTableRelatedClass
29+
}
30+
31+
class Conf extends DataFlow::Configuration {
32+
Conf() { this = "FlowToDataSerializerConstructor" }
33+
34+
override predicate isSource(DataFlow::Node node) { unsafeDataContractTypeCreation(node.asExpr()) }
35+
36+
override predicate isSink(DataFlow::Node node) { xmlSerializerConstructorArgument(node.asExpr()) }
37+
}
38+
39+
from Conf conf, DataFlow::Node source, DataFlow::Node sink
40+
where conf.hasFlow(source, sink)
41+
select sink,
42+
"Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source.",
43+
source, source.toString()

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<include src="DataSetSerialization.qhelp" /></qhelp>

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* @name XML deserialization with a type type derived from DataSet or DataTable
3+
* @description 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."
4+
* @kind problem
5+
* @problem.severity error
6+
* @precision medium
7+
* @id cs/dataset-serialization/xml-deserialization-with-dataset
8+
* @tags security
9+
*/
10+
11+
import csharp
12+
import DataSetSerialization
13+
14+
from UnsafeXmlReadMethodCall mc
15+
select mc,
16+
"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."

0 commit comments

Comments
 (0)