mirror of
https://github.com/github/codeql.git
synced 2026-04-25 08:45:14 +02:00
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
This commit is contained in:
Raul Garcia (MSFT)
@@ -0,0 +1,20 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<overview>
|
||||
<p>The <code>DataSet</code> and <code>DataTable</code> types are legacy .NET components that allow representing data sets as managed objects.<p>
|
||||
|
||||
|
||||
</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>
|
||||
|
||||
</recommendation>
|
||||
<references>
|
||||
|
||||
<li>Microsoft Docs<a href="https://go.microsoft.com/fwlink/?linkid=2132227">DataSet and DataTable security guidance</a>.</li>
|
||||
|
||||
</references>
|
||||
</qhelp>
|
||||
@@ -0,0 +1,110 @@
|
||||
import csharp
|
||||
|
||||
/**
|
||||
* Abstract class thats depnds or inherits from DataSet and DataTable types.
|
||||
**/
|
||||
abstract class DataSetOrTableRelatedClass extends Class {
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the DataSet and DataTable types, or types derived from them.
|
||||
**/
|
||||
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")
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets a class that include a property or generic of type DataSet and DataTable
|
||||
*/
|
||||
class ClassWithDataSetOrTableMember extends DataSetOrTableRelatedClass {
|
||||
ClassWithDataSetOrTableMember() {
|
||||
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 |
|
||||
p = this.getAProperty() |
|
||||
p.getType() instanceof DataSetOrTable or
|
||||
p.getType().(ConstructedGeneric).getATypeArgument() instanceof DataSetOrTable
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Serializable types
|
||||
*/
|
||||
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")
|
||||
) or exists( Attribute a |
|
||||
a = this.getAnAttribute() |
|
||||
a.getType().getQualifiedName().toString() = "System.SerializableAttribute"
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
predicate isClassUnsafeXmlSerializerImplementation( SerializableClass c, Member m) {
|
||||
exists( Property p |
|
||||
m = p |
|
||||
p = c.getAProperty() and
|
||||
p.getType() instanceof DataSetOrTableRelatedClass
|
||||
) or exists ( AssignableMember am |
|
||||
am = m |
|
||||
( am = c.getAField() or am = c.getAMember() ) and
|
||||
am.getType() instanceof DataSetOrTableRelatedClass
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* It is unsafe to serilize DataSet and DataTable related types
|
||||
*/
|
||||
class UnsafeXmlSerializerImplementation extends SerializableClass {
|
||||
UnsafeXmlSerializerImplementation() {
|
||||
isClassUnsafeXmlSerializerImplementation( this, _ )
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Method that may be unsafe when used to serialize DataSet and DataTable related types
|
||||
*/
|
||||
class UnsafeXmlReadMethod extends Method {
|
||||
UnsafeXmlReadMethod() {
|
||||
this.getQualifiedName().toString() = "System.Data.DataTable.ReadXml" or
|
||||
this.getQualifiedName().toString() = "System.Data.DataTable.ReadXmlSchema" or
|
||||
this.getQualifiedName().toString() = "System.Data.DataSet.ReadXml" or
|
||||
this.getQualifiedName().toString() = "System.Data.DataSet.ReadXmlSchema" or
|
||||
(
|
||||
this.getName().matches("ReadXml%") and
|
||||
exists( Class c |
|
||||
c.getAMethod() = this |
|
||||
c.getABaseType*() instanceof DataSetOrTableRelatedClass or
|
||||
c.getABaseType*() instanceof DataSetOrTableRelatedClass
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* MethodCal that may be unsafe when used to serialize DataSet and DataTable related types
|
||||
*/
|
||||
class UnsafeXmlReadMethodCall extends MethodCall {
|
||||
UnsafeXmlReadMethodCall() {
|
||||
exists( UnsafeXmlReadMethod uxrm |
|
||||
uxrm.getACall() = this
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="DataSetSerialization.qhelp" /></qhelp>
|
||||
@@ -0,0 +1,15 @@
|
||||
/**
|
||||
* @name Defining a class that inherits or has a property derived from the obsolete DataSet or DataTable types
|
||||
* @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.
|
||||
* @kind problem
|
||||
* @problem.severity warning
|
||||
* @id cs/dataset-serialization/defining-dataset-related-type
|
||||
* @tags security
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import DataSetSerialization
|
||||
|
||||
from DataSetOrTableRelatedClass dstc
|
||||
where dstc.fromSource()
|
||||
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."
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="DataSetSerialization.qhelp" /></qhelp>
|
||||
@@ -0,0 +1,19 @@
|
||||
/**
|
||||
* @name Defining a potentially unsafe XML serializer
|
||||
* @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.
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @precision medium
|
||||
* @id cs/dataset-serialization/defining-potentially-unsafe-xml-serializer
|
||||
* @tags security
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import DataSetSerialization
|
||||
|
||||
from UnsafeXmlSerializerImplementation c, Member m
|
||||
where c.fromSource() and
|
||||
isClassUnsafeXmlSerializerImplementation( c, m)
|
||||
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.",
|
||||
c, c.toString(),
|
||||
m, m.toString()
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="DataSetSerialization.qhelp" /></qhelp>
|
||||
@@ -0,0 +1,55 @@
|
||||
/**
|
||||
* @name Unsafe type is used in data contract serializer
|
||||
* @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
|
||||
* @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) {
|
||||
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")
|
||||
)
|
||||
)
|
||||
}
|
||||
|
||||
predicate unsafeDataContractTypeCreation (Expr e) {
|
||||
exists(MethodCall gt |
|
||||
gt.getTarget().getName() = "GetType" and
|
||||
e = gt and
|
||||
isClassDependingOnDataSetOrTable(gt.getQualifier().getType())
|
||||
) or
|
||||
isClassDependingOnDataSetOrTable(e.(TypeofExpr).getTypeAccess().getTarget())
|
||||
}
|
||||
|
||||
class Conf extends DataFlow::Configuration {
|
||||
Conf() {
|
||||
this = "FlowToDataSerializerConstructor"
|
||||
}
|
||||
|
||||
override predicate isSource(DataFlow::Node node) {
|
||||
unsafeDataContractTypeCreation(node.asExpr())
|
||||
}
|
||||
|
||||
override predicate isSink(DataFlow::Node node) {
|
||||
xmlSerializerConstructorTypeParameter (node.asExpr())
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
from Conf conf, DataFlow::Node source, DataFlow::Node sink
|
||||
where conf.hasFlow(source, sink)
|
||||
select sink, "Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source.", source, source.toString()
|
||||
@@ -0,0 +1,5 @@
|
||||
<!DOCTYPE qhelp PUBLIC
|
||||
"-//Semmle//qhelp//EN"
|
||||
"qhelp.dtd">
|
||||
<qhelp>
|
||||
<include src="DataSetSerialization.qhelp" /></qhelp>
|
||||
@@ -0,0 +1,16 @@
|
||||
/**
|
||||
* @name XML deserialization with a type type derived from DataSet or DataTable
|
||||
* @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."
|
||||
* @kind problem
|
||||
* @problem.severity error
|
||||
* @precision medium
|
||||
* @id cs/dataset-serialization/xml-deserialization-with-dataset
|
||||
* @tags security
|
||||
*/
|
||||
|
||||
import csharp
|
||||
import DataSetSerialization
|
||||
|
||||
from UnsafeXmlReadMethodCall mc, Method m
|
||||
where 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."
|
||||
@@ -0,0 +1,2 @@
|
||||
| test0.cs:13:18:13:43 | DerivesFromDeprecatedType1 | 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 deatils. |
|
||||
| test0.cs:59:18:59:38 | AttributeSerializer01 | 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 deatils. |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/Serialization/DefiningDatasetRelatedType.ql
|
||||
@@ -0,0 +1,2 @@
|
||||
| test0.cs:15:24:15:32 | MyDataSet | 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 deatils. | test0.cs:13:18:13:43 | DerivesFromDeprecatedType1 | DerivesFromDeprecatedType1 | test0.cs:15:24:15:32 | MyDataSet | MyDataSet |
|
||||
| test0.cs:61:25:61:33 | MyDataSet | 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 deatils. | test0.cs:59:18:59:38 | AttributeSerializer01 | AttributeSerializer01 | test0.cs:61:25:61:33 | MyDataSet | MyDataSet |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/Serialization/DefiningPotentiallyUnsafeXmlSerializer.ql
|
||||
@@ -0,0 +1,2 @@
|
||||
| test0.cs:95:49:95:63 | typeof(...) | Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source. | test0.cs:95:49:95:63 | typeof(...) | typeof(...) |
|
||||
| test0.cs:96:49:96:77 | typeof(...) | Unsafe type is used in data contract serializer. Make sure $@ comes from the trusted source. | test0.cs:96:49:96:77 | typeof(...) | typeof(...) |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/Serialization/UnsafeTypeUsedDataContractSerializer.ql
|
||||
@@ -0,0 +1 @@
|
||||
| test0.cs:88:17:88:46 | call to method ReadXmlSchema | 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 deatils. |
|
||||
@@ -0,0 +1 @@
|
||||
Security Features/Serialization/XmlDeserializationWithDataSet.ql
|
||||
@@ -0,0 +1 @@
|
||||
optimize: true
|
||||
@@ -0,0 +1,101 @@
|
||||
// semmle-extractor-options: /r:System.Data.Common.dll /r:System.Runtime.WindowsRuntime.dll /r:System.Xml.XmlSerializer.dll /r:System.Runtime.Serialization.Xml.dll /r:System.Runtime.Serialization.Xml.dll /r:System.Collections.dll /r:System.Private.Xml.dll /r:System.Private.DataContractSerialization.dll /r:System.Runtime.Extensions.dll /r:System.ComponentModel.TypeConverter.dll /r:System.Xml.ReaderWriter.dll /r:System.IO.FileSystem.dll
|
||||
|
||||
using System;
|
||||
using System.Data;
|
||||
using System.IO;
|
||||
using System.Xml.Serialization;
|
||||
using System.Runtime.Serialization;
|
||||
using System.Xml;
|
||||
using System.Collections.Generic;
|
||||
|
||||
namespace DataSetSerializationTest
|
||||
{
|
||||
public class DerivesFromDeprecatedType1 : XmlSerializer // bug
|
||||
{
|
||||
public DataSet MyDataSet { get; set; } // bug
|
||||
|
||||
public DerivesFromDeprecatedType1()
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* TODO: I cannot use DataContract on a QL unit test
|
||||
*
|
||||
[DataContract(Name = "Customer", Namespace = "http://www.contoso.com")]
|
||||
public class PatternDataContractSerializer : XmlObjectSerializer
|
||||
{
|
||||
[DataMember()]
|
||||
public DataSet MyDataSet { get; set; }
|
||||
[DataMember()]
|
||||
public DataTable MyDataTable { get; set; }
|
||||
|
||||
PatternDataContractSerializer() { }
|
||||
private ExtensionDataObject extensionData_Value;
|
||||
public ExtensionDataObject ExtensionData
|
||||
{
|
||||
get
|
||||
{
|
||||
return extensionData_Value;
|
||||
}
|
||||
set
|
||||
{
|
||||
extensionData_Value = value;
|
||||
}
|
||||
}
|
||||
|
||||
public override void WriteObject(System.IO.Stream stream, object graph) { }
|
||||
public override void WriteObjectContent(System.Xml.XmlDictionaryWriter writer, object graph) { }
|
||||
public override bool IsStartObject(System.Xml.XmlDictionaryReader reader) { return false; }
|
||||
public override void WriteStartObject(System.Xml.XmlDictionaryWriter writer, object graph) { }
|
||||
public override void WriteEndObject(System.Xml.XmlWriter writer) { }
|
||||
public override void WriteEndObject(XmlDictionaryWriter writer) { }
|
||||
public override object ReadObject(System.IO.Stream stream) { return null; }
|
||||
public override object ReadObject(XmlDictionaryReader reader, bool b) { return null; }
|
||||
}
|
||||
*/
|
||||
|
||||
[Serializable()]
|
||||
public class AttributeSerializer01 // bug
|
||||
{
|
||||
private DataSet MyDataSet; // bug
|
||||
|
||||
AttributeSerializer01()
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
class Program
|
||||
{
|
||||
static string GetSerializedDataSet(DataSet dataSet)
|
||||
{
|
||||
DataTable dataTable = new DataTable("MyTable");
|
||||
dataTable.Columns.Add("FirstName", typeof(string));
|
||||
dataTable.Columns.Add("LastName", typeof(string));
|
||||
dataTable.Columns.Add("Age", typeof(int));
|
||||
|
||||
StringWriter writer = new StringWriter();
|
||||
dataSet.WriteXml(writer, XmlWriteMode.DiffGram);
|
||||
return writer.ToString();
|
||||
}
|
||||
|
||||
static void datatable_readxmlschema_01(string fileName)
|
||||
{
|
||||
using (FileStream fs = File.OpenRead(fileName))
|
||||
{
|
||||
DataTable newTable = new DataTable();
|
||||
System.Xml.XmlTextReader reader = new System.Xml.XmlTextReader(fs);
|
||||
newTable.ReadXmlSchema(reader); //bug
|
||||
}
|
||||
}
|
||||
|
||||
static void Main(string[] args)
|
||||
{
|
||||
|
||||
XmlSerializer x = new XmlSerializer(typeof(DataSet)); // bug
|
||||
XmlSerializer y = new XmlSerializer(typeof(AttributeSerializer01)); //bug
|
||||
|
||||
Console.WriteLine("Hello World!");
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user