Skip to content

Commit 3e0481b

Browse files
raulgarciamsftraulgarciamsft
committed
Queries to help on the detection based on misuse of DataSet and DataTable serialization that could lead to security problems.
https://go.microsoft.com/fwlink/?linkid=2132227
1 parent 896cdf9 commit 3e0481b

20 files changed

+368
-0
lines changed

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
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 allow representing data sets as managed objects.<p>
7+
8+
9+
</overview>
10+
<recommendation>
11+
12+
<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>
13+
14+
</recommendation>
15+
<references>
16+
17+
<li>Microsoft Docs<a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a>.</li>
18+
19+
</references>
20+
</qhelp>

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

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

‎csharp/ql/src/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/Security Features/Serialization/DefiningDatasetRelatedType.ql

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
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, "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/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/Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
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 c.fromSource() and
16+
isClassUnsafeXmlSerializerImplementation( c, m)
17+
select m, "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.",
18+
c, c.toString(),
19+
m, m.toString()

‎csharp/ql/src/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/Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
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 high
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 isClassDependingOnDataSetOrTable( Class c ) {
15+
c instanceof DataSetOrTableRelatedClass
16+
}
17+
18+
predicate xmlSerializerConstructorTypeParameter (Expr e) {
19+
exists (ObjectCreation oc, Constructor c |
20+
e = oc.getArgument(0) |
21+
c = oc.getTarget() and
22+
(
23+
c.getDeclaringType().hasQualifiedName("System.Xml.Serialization.XmlSerializer") or
24+
c.getDeclaringType().getABaseType*().hasQualifiedName("System.Xml.Serialization.XmlSerializer")
25+
)
26+
)
27+
}
28+
29+
predicate unsafeDataContractTypeCreation (Expr e) {
30+
exists(MethodCall gt |
31+
gt.getTarget().getName() = "GetType" and
32+
e = gt and
33+
isClassDependingOnDataSetOrTable(gt.getQualifier().getType())
34+
) or
35+
isClassDependingOnDataSetOrTable(e.(TypeofExpr).getTypeAccess().getTarget())
36+
}
37+
38+
class Conf extends DataFlow::Configuration {
39+
Conf() {
40+
this = "FlowToDataSerializerConstructor"
41+
}
42+
43+
override predicate isSource(DataFlow::Node node) {
44+
unsafeDataContractTypeCreation(node.asExpr())
45+
}
46+
47+
override predicate isSink(DataFlow::Node node) {
48+
xmlSerializerConstructorTypeParameter (node.asExpr())
49+
}
50+
}
51+
52+
53+
from Conf conf, DataFlow::Node source, DataFlow::Node sink
54+
where conf.hasFlow(source, sink)
55+
select sink, "Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source.", source, source.toString()

‎csharp/ql/src/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/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, Method m
15+
where m.getACall() = mc
16+
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."

0 commit comments

Comments
 (0)