Merge pull request #12894 from michaelnebel/csharp/untrustedinput

C#: Re-factor the UnsafeDeserializationQuery to use the new API.
This commit is contained in:
Michael Nebel
2023-05-03 20:12:44 +02:00
committed by GitHub
3 changed files with 187 additions and 90 deletions

View File

@@ -23,11 +23,15 @@ abstract class Sink extends DataFlow::Node { }
*/
abstract private class InstanceMethodSink extends Sink {
InstanceMethodSink() {
not exists(
SafeConstructorTrackingConfig safeConstructorTracking, DataFlow::Node safeTypeUsage,
MethodCall mc
|
safeConstructorTracking.hasFlow(_, safeTypeUsage) and
not exists(DataFlow::Node safeTypeUsage, MethodCall mc |
(
DataContractJsonSafeConstructorTracking::flowTo(safeTypeUsage) or
JavaScriptSerializerSafeConstructorTracking::flowTo(safeTypeUsage) or
XmlObjectSerializerDerivedConstructorTracking::flowTo(safeTypeUsage) or
XmlSerializerSafeConstructorTracking::flowTo(safeTypeUsage) or
DataContractSerializerSafeConstructorTracking::flowTo(safeTypeUsage) or
XmlMessageFormatterSafeConstructorTracking::flowTo(safeTypeUsage)
) and
mc.getQualifier() = safeTypeUsage.asExpr() and
mc.getAnArgument() = this.asExpr()
)
@@ -47,9 +51,11 @@ abstract class Sanitizer extends DataFlow::Node { }
private class RemoteSource extends Source instanceof RemoteFlowSource { }
/**
* DEPRECATED: Use `TaintToObjectMethodTracking` instead.
*
* User input to object method call deserialization flow tracking.
*/
class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
deprecated class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
TaintToObjectMethodTrackingConfig() { this = "TaintToObjectMethodTrackingConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -60,9 +66,27 @@ class TaintToObjectMethodTrackingConfig extends TaintTracking::Configuration {
}
/**
* User input to object method call deserialization flow tracking configuration.
*/
private module TaintToObjectMethodTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }
predicate isSink(DataFlow::Node sink) { sink instanceof InstanceMethodSink }
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}
/**
* User input to object method call deserialization flow tracking module.
*/
module TaintToObjectMethodTracking = TaintTracking::Global<TaintToObjectMethodTrackingConfig>;
/**
* DEPRECATED: Use `JsonConvertTracking` instead.
*
* User input to `JsonConvert` call deserialization flow tracking.
*/
class JsonConvertTrackingConfig extends TaintTracking::Configuration {
deprecated class JsonConvertTrackingConfig extends TaintTracking::Configuration {
JsonConvertTrackingConfig() { this = "JsonConvertTrackingConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -74,6 +98,24 @@ class JsonConvertTrackingConfig extends TaintTracking::Configuration {
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
}
/**
* User input to `JsonConvert` call deserialization flow tracking configuration.
*/
private module JsonConvertTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }
predicate isSink(DataFlow::Node sink) {
sink instanceof NewtonsoftJsonConvertDeserializeObjectMethodSink
}
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}
/**
* User input to `JsonConvert` call deserialization flow tracking module.
*/
module JsonConvertTracking = TaintTracking::Global<JsonConvertTrackingConfig>;
/**
* DEPRECATED: Use `TypeNameTracking` instead.
*
@@ -186,9 +228,12 @@ private module TypeNameTrackingConfig implements DataFlow::ConfigSig {
module TypeNameTracking = DataFlow::Global<TypeNameTrackingConfig>;
/**
* DEPRECATED: Use `TaintToConstructorOrStaticMethodTracking` instead.
*
* User input to static method or constructor call deserialization flow tracking.
*/
class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Configuration {
deprecated class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Configuration
{
TaintToConstructorOrStaticMethodTrackingConfig() {
this = "TaintToConstructorOrStaticMethodTrackingConfig"
}
@@ -201,9 +246,28 @@ class TaintToConstructorOrStaticMethodTrackingConfig extends TaintTracking::Conf
}
/**
* User input to static method or constructor call deserialization flow tracking configuration.
*/
private module TaintToConstructorOrStaticMethodTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }
predicate isSink(DataFlow::Node sink) { sink instanceof ConstructorOrStaticMethodSink }
predicate isBarrier(DataFlow::Node node) { node instanceof Sanitizer }
}
/**
* User input to static method or constructor call deserialization flow tracking module.
*/
module TaintToConstructorOrStaticMethodTracking =
TaintTracking::Global<TaintToConstructorOrStaticMethodTrackingConfig>;
/**
* DEPRECATED: Use `TaintToObjectTypeTracking` instead.
*
* User input to instance type flow tracking.
*/
class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
deprecated class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
TaintToObjectTypeTrackingConfig() { this = "TaintToObjectTypeTrackingConfig" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
@@ -234,9 +298,47 @@ class TaintToObjectTypeTrackingConfig extends TaintTracking2::Configuration {
}
/**
* User input to instance type flow tracking config.
*/
private module TaintToObjectTypeTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) { source instanceof Source }
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
mc.getTarget() instanceof UnsafeDeserializer and
sink.asExpr() = mc.getQualifier()
)
}
predicate isAdditionalFlowStep(DataFlow::Node n1, DataFlow::Node n2) {
exists(MethodCall mc, Method m |
m = mc.getTarget() and
m.getDeclaringType().hasQualifiedName("System", "Type") and
m.hasName("GetType") and
m.isStatic() and
n1.asExpr() = mc.getArgument(0) and
n2.asExpr() = mc
)
or
exists(ObjectCreation oc |
n1.asExpr() = oc.getAnArgument() and
n2.asExpr() = oc and
oc.getObjectType() instanceof StrongTypeDeserializer
)
}
}
/**
* User input to instance type flow tracking module.
*/
module TaintToObjectTypeTracking = TaintTracking::Global<TaintToObjectTypeTrackingConfig>;
/**
* DEPRECATED: Use `WeakTypeCreationToUsageTracking` instead.
*
* Unsafe deserializer creation to usage tracking config.
*/
class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuration {
deprecated class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuration {
WeakTypeCreationToUsageTrackingConfig() { this = "DeserializerCreationToUsageTrackingConfig" }
override predicate isSource(DataFlow::Node source) {
@@ -255,13 +357,30 @@ class WeakTypeCreationToUsageTrackingConfig extends TaintTracking2::Configuratio
}
/**
* Safe deserializer creation to usage tracking config.
* Unsafe deserializer creation to usage tracking config.
*/
abstract class SafeConstructorTrackingConfig extends TaintTracking2::Configuration {
bindingset[this]
SafeConstructorTrackingConfig() { any() }
private module WeakTypeCreationToUsageTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc.getObjectType() instanceof WeakTypeDeserializer and
source.asExpr() = oc
)
}
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
mc.getTarget() instanceof UnsafeDeserializer and
sink.asExpr() = mc.getQualifier()
)
}
}
/**
* Unsafe deserializer creation to usage tracking module.
*/
module WeakTypeCreationToUsageTracking =
TaintTracking::Global<WeakTypeCreationToUsageTrackingConfig>;
/** BinaryFormatter */
private predicate isBinaryFormatterCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -367,13 +486,8 @@ private class DataContractJsonSerializerDeserializeMethodSink extends DataContra
}
}
private class DataContractJsonSafeConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
DataContractJsonSafeConstructorTrackingConfiguration() {
this = "DataContractJsonSafeConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module DataContractJsonSafeConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(Constructor c |
@@ -385,7 +499,7 @@ private class DataContractJsonSafeConstructorTrackingConfiguration extends SafeC
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isDataContractJsonSerializerCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -393,6 +507,9 @@ private class DataContractJsonSafeConstructorTrackingConfiguration extends SafeC
}
}
private module DataContractJsonSafeConstructorTracking =
TaintTracking::Global<DataContractJsonSafeConstructorTrackingConfig>;
/** JavaScriptSerializer */
private predicate isJavaScriptSerializerCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -417,13 +534,8 @@ private class JavaScriptSerializerDeserializeMethodSink extends JavaScriptSerial
}
}
private class JavaScriptSerializerSafeConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
JavaScriptSerializerSafeConstructorTrackingConfiguration() {
this = "JavaScriptSerializerSafeConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module JavaScriptSerializerSafeConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(Constructor c |
@@ -434,7 +546,7 @@ private class JavaScriptSerializerSafeConstructorTrackingConfiguration extends S
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isJavaScriptSerializerCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -442,6 +554,9 @@ private class JavaScriptSerializerSafeConstructorTrackingConfiguration extends S
}
}
private module JavaScriptSerializerSafeConstructorTracking =
TaintTracking::Global<JavaScriptSerializerSafeConstructorTrackingConfig>;
/** XmlObjectSerializer */
private predicate isXmlObjectSerializerCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -461,13 +576,8 @@ private class XmlObjectSerializerDeserializeMethodSink extends XmlObjectSerializ
}
}
private class XmlObjectSerializerDerivedConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
XmlObjectSerializerDerivedConstructorTrackingConfiguration() {
this = "XmlObjectSerializerDerivedConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module XmlObjectSerializerDerivedConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(ValueOrRefType declaringType |
@@ -481,7 +591,7 @@ private class XmlObjectSerializerDerivedConstructorTrackingConfiguration extends
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isXmlObjectSerializerCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -489,6 +599,9 @@ private class XmlObjectSerializerDerivedConstructorTrackingConfiguration extends
}
}
private module XmlObjectSerializerDerivedConstructorTracking =
TaintTracking::Global<XmlObjectSerializerDerivedConstructorTrackingConfig>;
/** XmlSerializer */
private predicate isXmlSerializerCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -507,13 +620,8 @@ private class XmlSerializerDeserializeMethodSink extends XmlSerializerSink {
}
}
private class XmlSerializerSafeConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
XmlSerializerSafeConstructorTrackingConfiguration() {
this = "XmlSerializerSafeConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module XmlSerializerSafeConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(Constructor c |
@@ -525,7 +633,7 @@ private class XmlSerializerSafeConstructorTrackingConfiguration extends SafeCons
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isXmlSerializerCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -533,6 +641,9 @@ private class XmlSerializerSafeConstructorTrackingConfiguration extends SafeCons
}
}
private module XmlSerializerSafeConstructorTracking =
TaintTracking::Global<XmlSerializerSafeConstructorTrackingConfig>;
/** DataContractSerializer */
private predicate isDataContractSerializerCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -555,13 +666,8 @@ private class DataContractSerializerDeserializeMethodSink extends DataContractSe
}
}
private class DataContractSerializerSafeConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
DataContractSerializerSafeConstructorTrackingConfiguration() {
this = "DataContractSerializerSafeConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module DataContractSerializerSafeConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(Constructor c |
@@ -573,7 +679,7 @@ private class DataContractSerializerSafeConstructorTrackingConfiguration extends
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isDataContractSerializerCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -581,6 +687,9 @@ private class DataContractSerializerSafeConstructorTrackingConfiguration extends
}
}
private module DataContractSerializerSafeConstructorTracking =
TaintTracking::Global<DataContractSerializerSafeConstructorTrackingConfig>;
/** XmlMessageFormatter */
private predicate isXmlMessageFormatterCall(MethodCall mc, Method m) {
m = mc.getTarget() and
@@ -599,13 +708,8 @@ private class XmlMessageFormatterDeserializeMethodSink extends XmlMessageFormatt
}
}
private class XmlMessageFormatterSafeConstructorTrackingConfiguration extends SafeConstructorTrackingConfig
{
XmlMessageFormatterSafeConstructorTrackingConfiguration() {
this = "XmlMessageFormatterSafeConstructorTrackingConfiguration"
}
override predicate isSource(DataFlow::Node source) {
private module XmlMessageFormatterSafeConstructorTrackingConfig implements DataFlow::ConfigSig {
predicate isSource(DataFlow::Node source) {
exists(ObjectCreation oc |
oc = source.asExpr() and
exists(Constructor c |
@@ -617,7 +721,7 @@ private class XmlMessageFormatterSafeConstructorTrackingConfiguration extends Sa
)
}
override predicate isSink(DataFlow::Node sink) {
predicate isSink(DataFlow::Node sink) {
exists(MethodCall mc |
isXmlMessageFormatterCall(mc, _) and
mc.getQualifier() = sink.asExpr()
@@ -625,6 +729,9 @@ private class XmlMessageFormatterSafeConstructorTrackingConfiguration extends Sa
}
}
private module XmlMessageFormatterSafeConstructorTracking =
TaintTracking::Global<XmlMessageFormatterSafeConstructorTrackingConfig>;
/** LosFormatter */
private predicate isLosFormatterCall(MethodCall mc, Method m) {
m = mc.getTarget() and

View File

@@ -13,43 +13,40 @@
import csharp
import semmle.code.csharp.security.dataflow.UnsafeDeserializationQuery
import DataFlow::PathGraph
import Flow::PathGraph
from DataFlow::PathNode userInput, DataFlow::PathNode deserializeCallArg
module Flow =
DataFlow::MergePathGraph3<TaintToObjectMethodTracking::PathNode,
TaintToConstructorOrStaticMethodTracking::PathNode, JsonConvertTracking::PathNode,
TaintToObjectMethodTracking::PathGraph, TaintToConstructorOrStaticMethodTracking::PathGraph,
JsonConvertTracking::PathGraph>;
from Flow::PathNode userInput, Flow::PathNode deserializeCallArg
where
exists(TaintToObjectMethodTrackingConfig taintTracking |
// all flows from user input to deserialization with weak and strong type serializers
taintTracking.hasFlowPath(userInput, deserializeCallArg)
) and
// all flows from user input to deserialization with weak and strong type serializers
TaintToObjectMethodTracking::flowPath(userInput.asPathNode1(), deserializeCallArg.asPathNode1()) and
// intersect with strong types, but user controlled or weak types deserialization usages
(
exists(
DataFlow::Node weakTypeUsage,
WeakTypeCreationToUsageTrackingConfig weakTypeDeserializerTracking, MethodCall mc
|
weakTypeDeserializerTracking.hasFlowTo(weakTypeUsage) and
exists(DataFlow::Node weakTypeUsage, MethodCall mc |
WeakTypeCreationToUsageTracking::flowTo(weakTypeUsage) and
mc.getQualifier() = weakTypeUsage.asExpr() and
mc.getAnArgument() = deserializeCallArg.getNode().asExpr()
)
or
exists(
TaintToObjectTypeTrackingConfig userControlledTypeTracking, DataFlow::Node taintedTypeUsage,
MethodCall mc
|
userControlledTypeTracking.hasFlowTo(taintedTypeUsage) and
exists(DataFlow::Node taintedTypeUsage, MethodCall mc |
TaintToObjectTypeTracking::flowTo(taintedTypeUsage) and
mc.getQualifier() = taintedTypeUsage.asExpr() and
mc.getAnArgument() = deserializeCallArg.getNode().asExpr()
)
)
or
// no type check needed - straightforward taint -> sink
exists(TaintToConstructorOrStaticMethodTrackingConfig taintTracking2 |
taintTracking2.hasFlowPath(userInput, deserializeCallArg)
)
TaintToConstructorOrStaticMethodTracking::flowPath(userInput.asPathNode2(),
deserializeCallArg.asPathNode2())
or
// JsonConvert static method call, but with additional unsafe typename tracking
exists(JsonConvertTrackingConfig taintTrackingJsonConvert, DataFlow::Node settingsCallArg |
taintTrackingJsonConvert.hasFlowPath(userInput, deserializeCallArg) and
exists(DataFlow::Node settingsCallArg |
JsonConvertTracking::flowPath(userInput.asPathNode3(), deserializeCallArg.asPathNode3()) and
TypeNameTracking::flow(_, settingsCallArg) and
deserializeCallArg.getNode().asExpr().getParent() = settingsCallArg.asExpr().getParent()
)

View File

@@ -1,19 +1,12 @@
edges
| ../../../../resources/stubs/Newtonsoft.Json/13.0.1/Newtonsoft.Json.cs:930:20:930:20 | 4 : Int32 | Test.cs:19:32:19:52 | access to constant Auto : Int32 |
| Test.cs:9:46:9:49 | access to parameter data : TextBox | Test.cs:9:46:9:54 | access to property Text |
| Test.cs:17:46:17:49 | access to parameter data : TextBox | Test.cs:17:46:17:54 | access to property Text |
| Test.cs:19:32:19:52 | access to constant Auto : Int32 | Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings |
| Test.cs:19:32:19:52 | access to constant Auto : TypeNameHandling | Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings |
| Test.cs:25:46:25:49 | access to parameter data : TextBox | Test.cs:25:46:25:54 | access to property Text |
nodes
| ../../../../resources/stubs/Newtonsoft.Json/13.0.1/Newtonsoft.Json.cs:930:20:930:20 | 4 : Int32 | semmle.label | 4 : Int32 |
| Test.cs:9:46:9:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
| Test.cs:9:46:9:54 | access to property Text | semmle.label | access to property Text |
| Test.cs:17:46:17:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
| Test.cs:17:46:17:54 | access to property Text | semmle.label | access to property Text |
| Test.cs:17:57:20:9 | object creation of type JsonSerializerSettings | semmle.label | object creation of type JsonSerializerSettings |
| Test.cs:19:32:19:52 | access to constant Auto : Int32 | semmle.label | access to constant Auto : Int32 |
| Test.cs:19:32:19:52 | access to constant Auto : TypeNameHandling | semmle.label | access to constant Auto : TypeNameHandling |
| Test.cs:25:46:25:49 | access to parameter data : TextBox | semmle.label | access to parameter data : TextBox |
| Test.cs:25:46:25:54 | access to property Text | semmle.label | access to property Text |
subpaths