Skip to content

Commit 49336e3

Browse files
chanel-ychanel-y
committed
added initial unsafe typenamehandling queries
1 parent 9bafbaf commit 49336e3

5 files changed

Lines changed: 344 additions & 0 deletions

File tree

‎csharp/ql/lib/semmle/code/csharp/security/dataflow/TypeNameHandlingQuery.qll‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import csharp
2+
import semmle.code.csharp.security.dataflow.flowsources.Remote
3+
import semmle.code.csharp.serialization.Deserializers
4+
5+
class BadTypeHandling extends Expr {
6+
BadTypeHandling() {
7+
exists(Enum e, EnumConstant c |
8+
e.hasFullyQualifiedName("Newtonsoft.Json", "TypeNameHandling") and
9+
c = e.getAnEnumConstant() and
10+
this = c.getAnAccess() and
11+
not c.hasName("None")
12+
) or
13+
this.(IntegerLiteral).getValue().toInt() > 0
14+
}
15+
}
16+
17+
class TypeNameHandlingProperty extends Property {
18+
TypeNameHandlingProperty() {
19+
this.hasFullyQualifiedName("Newtonsoft.Json", ["JsonSerializerSettings", "JsonSerializer"], "TypeNameHandling")
20+
}
21+
}
22+
23+
class TypeNameHandlingPropertyWrite extends PropertyWrite {
24+
TypeNameHandlingPropertyWrite() { this.getProperty() instanceof TypeNameHandlingProperty }
25+
26+
Expr getAssignedValue() {
27+
exists(AssignExpr a |
28+
a.getLValue() = this and
29+
result = a.getRValue()
30+
)
31+
}
32+
}
33+
34+
class BadTypeHandlingPropertyWrite extends TypeNameHandlingPropertyWrite {
35+
BadTypeHandlingPropertyWrite() {
36+
exists(BadTypeHandling b |
37+
DataFlow::localExprFlow(b, getAssignedValue())
38+
)
39+
}
40+
}
41+
42+
class BinderPropertyWrite extends PropertyWrite {
43+
BinderPropertyWrite() {
44+
this.getProperty().hasFullyQualifiedName("Newtonsoft.Json", ["JsonSerializerSettings", "JsonSerializer"], ["SerializationBinder", "Binder"])
45+
}
46+
}
47+
48+
module UserInputToDeserializeObjectCallConfig implements DataFlow::ConfigSig {
49+
predicate isSource(DataFlow::Node source) {
50+
source instanceof RemoteFlowSource
51+
}
52+
53+
predicate isSink(DataFlow::Node sink) {
54+
exists(MethodCall mc |
55+
mc.getTarget().hasFullyQualifiedName("Newtonsoft.Json.JsonConvert", _) and
56+
mc.getTarget().hasUndecoratedName("DeserializeObject") and
57+
sink.asExpr() = mc.getAnArgument()
58+
)
59+
}
60+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ){
61+
exists(MethodCall ma |
62+
ma.getTarget().hasName("ToString") and
63+
ma.getQualifier() = pred.asExpr() and
64+
succ.asExpr() = ma
65+
)
66+
}
67+
}
68+
69+
module UserInputToDeserializeObjectCallFlow = TaintTracking::Global<UserInputToDeserializeObjectCallConfig>;
70+
71+
72+
module UnsafeTypeNameHandlingFlowConfig implements DataFlow::ConfigSig {
73+
predicate isSource(DataFlow::Node source) {
74+
source.asExpr() instanceof BadTypeHandling
75+
}
76+
77+
predicate isSink(DataFlow::Node sink) {
78+
exists(TypeNameHandlingPropertyWrite prop | sink = DataFlow::exprNode(prop.getAssignedValue()))
79+
}
80+
81+
predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
82+
}
83+
84+
module UnsafeTypeNameHandlingFlow = DataFlow::Global<UnsafeTypeNameHandlingFlowConfig>;
85+
86+
/**
87+
* An ObjectCreation of type `Newtonson.Json.JsonSerializerSettings.JsonSerializerSettings` or `Newtonson.Json.JsonSerializerSettings.JsonSerializer`.
88+
*/
89+
class JsonSerializerSettingsCreation extends ObjectCreation {
90+
JsonSerializerSettingsCreation() {
91+
this.getTarget()
92+
.hasFullyQualifiedName("Newtonsoft.Json.JsonSerializerSettings", "JsonSerializerSettings") or
93+
this.getTarget()
94+
.hasFullyQualifiedName("Newtonsoft.Json.JsonSerializer", "JsonSerializer")
95+
}
96+
Class getAssignedBinderType(){
97+
exists(AssignExpr ae |
98+
ae.getLValue() = this.getBinderPropertyWrite() and
99+
ae.getRValue().getType() = result
100+
)
101+
}
102+
103+
BinderPropertyWrite getBinderPropertyWrite() {
104+
result = this.getPropertyWrite()
105+
}
106+
107+
TypeNameHandlingPropertyWrite getTypeNameHandlingPropertyWrite() {
108+
result = this.getPropertyWrite()
109+
}
110+
111+
PropertyWrite getPropertyWrite(){
112+
result = this.getInitializer().getAChild*()
113+
or
114+
// Direct local flow via some intermediary
115+
DataFlow::localExprFlow(this, result.getQualifier())
116+
or
117+
// Local flow via property writes
118+
hasPropertyWrite(result)
119+
}
120+
121+
/**
122+
* The qualifier to `pw` is a property, which this value flows into somewhere locally.
123+
* Janky local DF.
124+
* */
125+
bindingset[pw]
126+
pragma[inline_late]
127+
predicate hasPropertyWrite(PropertyWrite pw){
128+
exists(Assignment a |
129+
a.getRValue() = this
130+
and a.getLValue().(PropertyAccess).getTarget() = pw.getQualifier().(PropertyAccess).getTarget()
131+
and a.getEnclosingCallable() = pw.getEnclosingCallable()
132+
)
133+
}
134+
}

‎csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandling.ql‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/**
2+
* @name Unsafe TypeNameHandling used with an insecure binder implementation
3+
* @description Checks for when JsonSerializer that also uses an unsafe TypeNameHandling setting is used to deserialize user input
4+
* @id cs/unsafe-type-name-handling
5+
* @kind problem
6+
* @problem.severity warning
7+
* @precision high
8+
* @tags security
9+
* external/cwe/cwe-502
10+
*/
11+
12+
import csharp
13+
import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery
14+
15+
from
16+
JsonSerializerSettingsCreation oc, TypeNameHandlingPropertyWrite pw, DataFlow::Node source,
17+
DataFlow::Node sink
18+
where
19+
pw = oc.getTypeNameHandlingPropertyWrite() and
20+
sink = DataFlow::exprNode(pw.getAssignedValue()) and
21+
UnsafeTypeNameHandlingFlow::flow(source, sink)
22+
select oc.getAssignedBinderType().getAMethod("BindToType"),
23+
"This is the BindToType() implementation used in $@ with $@. Ensure that it checks the TypeName against an allow or deny list, and returns null or throws an exception when passed an unexpected type", oc, "JsonSerializerSettings",
24+
pw.getAssignedValue(), "an unsafe TypeNameHandling value"

‎csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInDefaultSetting.ql‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/**
2+
* @name Unsafe TypeNameHandling assigned to JsonConvert.DefaultSetting
3+
* @description Using an unsafe TypeNameHandling constant is a security vulnerability.
4+
* @kind problem
5+
* @id cs/unsafe-type-name-handling-default-setting
6+
* @problem.severity error
7+
* @precision high
8+
* @tags security
9+
* external/cwe/cwe-502
10+
*/
11+
12+
import csharp
13+
import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery
14+
import semmle.code.csharp.dataflow.internal.DataFlowPrivate as DataFlowPrivate
15+
16+
module TypeNameHandlingDefaultSettingsFlowConfig implements DataFlow::ConfigSig {
17+
predicate isSource(DataFlow::Node source) {
18+
source.asExpr() instanceof JsonSerializerSettingsCreation
19+
}
20+
21+
predicate isSink(DataFlow::Node sink) {
22+
exists(AssignExpr ae, PropertyWrite pw |
23+
pw.getProperty().hasFullyQualifiedName("Newtonsoft.Json.JsonConvert", "DefaultSettings") and
24+
sink.asExpr() = ae.getRValue() and
25+
pw = ae.getLValue()
26+
)
27+
}
28+
29+
predicate isAdditionalFlowStep(DataFlow::Node pred, DataFlow::Node succ) {
30+
// Step from the dataflow node representing the return value of the lambda to the lambda itself.
31+
exists(LambdaExpr le |
32+
pred.(DataFlowPrivate::ReturnNode).getEnclosingCallable() = le and
33+
le = succ.asExpr()
34+
)
35+
}
36+
}
37+
38+
module TypeNameHandlingDefaultSettingsFlow =
39+
TaintTracking::Global<TypeNameHandlingDefaultSettingsFlowConfig>;
40+
41+
from DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingPropertyWrite
42+
where
43+
TypeNameHandlingDefaultSettingsFlow::flow(badTypeNameHandling, typeNameHandlingPropertyWrite) and
44+
// Detecting if there exists some flow from user input to a call that could use the unsafe settings
45+
// Not including the actual flow in the results because doing so a cartesian product as the two flows are unrelated
46+
UserInputToDeserializeObjectCallFlow::flow(_, _)
47+
select badTypeNameHandling,
48+
"Assignment of this TypeNameHandling constant to JsonConvert.DefaultSettings is unsafe"

‎csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingInStartup.ql‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/**
2+
* @name Unsafe TypeNameHandling in Startup
3+
* @description Using an unsafe TypeNameHandling constant is a security vulnerability.
4+
* @kind problem
5+
* @id cs/unsafe-type-name-handling-in-startup
6+
* @problem.severity error
7+
* @precision high
8+
* @tags security
9+
* external/cwe/cwe-502
10+
*/
11+
12+
import csharp
13+
import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery
14+
15+
class AddJsonOptionsCall extends MethodCall {
16+
AddJsonOptionsCall() {
17+
this.getTarget()
18+
.hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection.NewtonsoftJsonMvcBuilderExtensions",
19+
"AddNewtonsoftJson") or
20+
this.getTarget()
21+
.hasFullyQualifiedName("Microsoft.Extensions.DependencyInjection.MvcJsonMvcBuilderExtensions",
22+
"AddJsonOptions")
23+
}
24+
}
25+
26+
from
27+
TypeNameHandlingPropertyWrite pw, AddJsonOptionsCall addJsonOptions,
28+
DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingPropertyWrite
29+
where
30+
addJsonOptions.getEnclosingCallable().hasName("ConfigureServices") and
31+
addJsonOptions.getAChild*() = pw and
32+
typeNameHandlingPropertyWrite = DataFlow::exprNode(pw.getAssignedValue()) and
33+
UnsafeTypeNameHandlingFlow::flow(badTypeNameHandling, typeNameHandlingPropertyWrite) and
34+
UserInputToDeserializeObjectCallFlow::flow(_, _)
35+
select badTypeNameHandling, "Use of this TypeNameHandling constant in Startup.cs is unsafe"

‎csharp/ql/src/Security Features/CWE-502/UnsafeTypeNameHandlingWithoutBinder.ql‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/**
2+
* @name Unsafe TypeNameHandling without Binder to deserialize user input
3+
* @description Using an unsafe TypeNameHandling constant is a security vulnerability.
4+
* @kind path-problem
5+
* @id cs/unsafe-type-name-handling-without-binder
6+
* @problem.severity error
7+
* @precision high
8+
* @tags security
9+
* external/cwe/cwe-502
10+
*/
11+
12+
import csharp
13+
import semmle.code.csharp.serialization.Deserializers
14+
import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery
15+
16+
class DeserializeArg extends Expr {
17+
MethodCall deserializeCall;
18+
DeserializeArg() {
19+
deserializeCall.getTarget() instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod and
20+
deserializeCall.getAnArgument() = this
21+
}
22+
MethodCall getDeserializeCall() {
23+
result = deserializeCall
24+
}
25+
}
26+
27+
class JsonSerializerSettingsArg extends DeserializeArg {
28+
JsonSerializerSettingsArg() {
29+
this.getType() instanceof JsonSerializerSettingsClass
30+
}
31+
}
32+
33+
predicate hasBinderSet(JsonSerializerSettingsArg arg) {
34+
// //passed as argument to initializer
35+
exists(BinderFlow::PathNode sink |
36+
sink.isSink() and
37+
sink.getNode().asExpr() = arg
38+
) or
39+
//set in later Propertywrite
40+
exists(PropertyWrite pw |
41+
pw.getProperty().hasName(["Binder", "SerializationBinder"]) and
42+
pw.getQualifier().(Access).getTarget() = arg.(Access).getTarget()
43+
)
44+
}
45+
46+
module BinderConfig implements DataFlow::ConfigSig {
47+
predicate isSource(DataFlow::Node source){
48+
exists(ObjectCreation oc, MemberInitializer mi |
49+
oc.getInitializer().(ObjectInitializer).getAMemberInitializer() = mi and
50+
mi.getInitializedMember().hasName(["Binder", "SerializationBinder"]) and
51+
source.asExpr() = oc
52+
)
53+
}
54+
predicate isSink(DataFlow::Node sink){
55+
sink.asExpr() instanceof JsonSerializerSettingsArg
56+
}
57+
}
58+
module BinderFlow = DataFlow::Global<BinderConfig>;
59+
60+
module TypeNameFlowConfig implements DataFlow::ConfigSig {
61+
predicate isSource(DataFlow::Node source) {
62+
source.asExpr() instanceof BadTypeHandling
63+
}
64+
65+
predicate isSink(DataFlow::Node sink) {
66+
exists(JsonSerializerSettingsArg arg |
67+
not hasBinderSet(arg) and
68+
sink.asExpr() = arg
69+
)
70+
}
71+
72+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
73+
node1.asExpr() instanceof IntegerLiteral and
74+
node2.asExpr().(CastExpr).getExpr() = node1.asExpr()
75+
or
76+
node1.getType() instanceof TypeNameHandlingEnum and
77+
exists(TypeNameHandlingPropertyWrite pw, Assignment a |
78+
a.getLValue() = pw and
79+
(
80+
node1.asExpr() = a.getRValue() and
81+
node2.asExpr() = pw.getQualifier()
82+
or
83+
exists(ObjectInitializer oi |
84+
node1.asExpr() = oi.getAMemberInitializer().getRValue() and
85+
node2.asExpr() = oi
86+
)
87+
)
88+
)
89+
}
90+
}
91+
92+
module TypeNameFlow = DataFlow::Global<TypeNameFlowConfig>;
93+
94+
from
95+
UserInputToDeserializeObjectCallFlow::PathNode userInput, UserInputToDeserializeObjectCallFlow::PathNode deserializeArg,
96+
DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingSettingArg,
97+
BadTypeHandling bth
98+
where
99+
TypeNameFlow::flow(badTypeNameHandling, typeNameHandlingSettingArg) and
100+
UserInputToDeserializeObjectCallFlow::flowPath(userInput, deserializeArg) and
101+
deserializeArg.getNode().asExpr().(DeserializeArg).getDeserializeCall() = typeNameHandlingSettingArg.asExpr().(JsonSerializerSettingsArg).getDeserializeCall() and
102+
bth = badTypeNameHandling.asExpr()
103+
select deserializeArg.getNode(), userInput, deserializeArg, "Use of $@ constant to deserialize user-controlled input is unsafe", bth, "this Json.net TypeNameHandling"

0 commit comments

Comments
 (0)