Skip to content

Commit a2d5377

Browse files
chanel-ychanel-y
committed
reorganize without-binder query to use the existing typenamehandling -> typenamehanlding property flow in the shared qll
1 parent 65fd69e commit a2d5377

3 files changed

Lines changed: 90 additions & 83 deletions

File tree

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

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class TypeNameHandlingPropertyWrite extends PropertyWrite {
3434
class BadTypeHandlingPropertyWrite extends TypeNameHandlingPropertyWrite {
3535
BadTypeHandlingPropertyWrite() {
3636
exists(BadTypeHandling b |
37-
DataFlow::localExprFlow(b, getAssignedValue())
37+
DataFlow::localExprFlow(b, this.getAssignedValue())
3838
)
3939
}
4040
}
@@ -68,17 +68,101 @@ module UserInputToDeserializeObjectCallConfig implements DataFlow::ConfigSig {
6868

6969
module UserInputToDeserializeObjectCallFlow = TaintTracking::Global<UserInputToDeserializeObjectCallConfig>;
7070

71+
module BinderConfig implements DataFlow::ConfigSig {
72+
predicate isSource(DataFlow::Node source){
73+
exists(ObjectCreation oc, MemberInitializer mi |
74+
oc.getInitializer().(ObjectInitializer).getAMemberInitializer() = mi and
75+
mi.getInitializedMember().hasName(["Binder", "SerializationBinder"]) and
76+
source.asExpr() = oc
77+
)
78+
}
79+
predicate isSink(DataFlow::Node sink){
80+
sink.asExpr() instanceof JsonSerializerSettingsArg
81+
}
82+
}
83+
module BinderFlow = DataFlow::Global<BinderConfig>;
84+
85+
class DeserializeArg extends Expr {
86+
MethodCall deserializeCall;
87+
DeserializeArg() {
88+
deserializeCall.getTarget() instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod and
89+
deserializeCall.getAnArgument() = this
90+
}
91+
MethodCall getDeserializeCall() {
92+
result = deserializeCall
93+
}
94+
}
95+
96+
class JsonSerializerSettingsArg extends DeserializeArg {
97+
JsonSerializerSettingsArg() {
98+
this.getType() instanceof JsonSerializerSettingsClass
99+
}
100+
}
101+
102+
predicate hasBinderSet(JsonSerializerSettingsArg arg) {
103+
// //passed as argument to initializer
104+
exists(BinderFlow::PathNode sink |
105+
sink.isSink() and
106+
sink.getNode().asExpr() = arg
107+
) or
108+
//set in later Propertywrite
109+
exists(PropertyWrite pw |
110+
pw.getProperty().hasName(["Binder", "SerializationBinder"]) and
111+
pw.getQualifier().(Access).getTarget() = arg.(Access).getTarget()
112+
)
113+
}
114+
115+
class TypeNameHandlingPropertySink extends DataFlow::Node {
116+
TypeNameHandlingPropertySink() {
117+
exists(TypeNameHandlingPropertyWrite pw |
118+
this.asExpr() = pw.getAssignedValue()
119+
)
120+
}
121+
122+
predicate hasBinderSet() {
123+
exists(JsonSerializerSettingsArg arg |
124+
this.asExpr() = arg and
125+
hasBinderSet(arg)
126+
)
127+
}
128+
129+
}
71130

72131
module UnsafeTypeNameHandlingFlowConfig implements DataFlow::ConfigSig {
73132
predicate isSource(DataFlow::Node source) {
74133
source.asExpr() instanceof BadTypeHandling
75134
}
76135

77136
predicate isSink(DataFlow::Node sink) {
78-
exists(TypeNameHandlingPropertyWrite prop | sink = DataFlow::exprNode(prop.getAssignedValue()))
137+
sink.asExpr() instanceof JsonSerializerSettingsArg
79138
}
80139

81140
predicate isBarrierIn(DataFlow::Node node) { isSource(node) }
141+
142+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
143+
node1.asExpr() instanceof IntegerLiteral and
144+
node2.asExpr().(CastExpr).getExpr() = node1.asExpr()
145+
or
146+
node1.getType() instanceof TypeNameHandlingEnum and
147+
exists(TypeNameHandlingPropertyWrite pw, Assignment a |
148+
a.getLValue() = pw and
149+
(
150+
// Explicit property write: flow from the assigned value to the JsonSerializerSettingsArg
151+
// that accesses the same settings variable
152+
node1.asExpr() = a.getRValue() and
153+
node2.asExpr().(JsonSerializerSettingsArg).(VariableAccess).getTarget() =
154+
pw.getQualifier().(VariableAccess).getTarget()
155+
or
156+
// ObjectInitializer case: flow from the member initializer value to the
157+
// ObjectCreation, which then flows locally to the JsonSerializerSettingsArg
158+
exists(ObjectInitializer oi, ObjectCreation oc |
159+
node1.asExpr() = oi.getAMemberInitializer().getRValue() and
160+
oc.getInitializer() = oi and
161+
DataFlow::localExprFlow(oc, node2.asExpr().(JsonSerializerSettingsArg))
162+
)
163+
)
164+
)
165+
}
82166
}
83167

84168
module UnsafeTypeNameHandlingFlow = DataFlow::Global<UnsafeTypeNameHandlingFlowConfig>;

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
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
2+
* @name Unsafe TypeNameHandling value
3+
* @description Checks for when JsonSerializer that also uses an unsafe TypeNameHandling setting
44
* @id cs/unsafe-type-name-handling
55
* @kind problem
66
* @problem.severity warning

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

Lines changed: 2 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -13,92 +13,15 @@ import csharp
1313
import semmle.code.csharp.serialization.Deserializers
1414
import semmle.code.csharp.security.dataflow.TypeNameHandlingQuery
1515

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-
9416
import UserInputToDeserializeObjectCallFlow::PathGraph
9517

9618
from
9719
UserInputToDeserializeObjectCallFlow::PathNode userInput, UserInputToDeserializeObjectCallFlow::PathNode deserializeArg,
9820
DataFlow::Node badTypeNameHandling, DataFlow::Node typeNameHandlingSettingArg,
9921
BadTypeHandling bth
10022
where
101-
TypeNameFlow::flow(badTypeNameHandling, typeNameHandlingSettingArg) and
23+
UnsafeTypeNameHandlingFlow::flow(badTypeNameHandling, typeNameHandlingSettingArg) and
24+
not hasBinderSet(typeNameHandlingSettingArg.asExpr().(JsonSerializerSettingsArg)) and
10225
UserInputToDeserializeObjectCallFlow::flowPath(userInput, deserializeArg) and
10326
deserializeArg.getNode().asExpr().(DeserializeArg).getDeserializeCall() = typeNameHandlingSettingArg.asExpr().(JsonSerializerSettingsArg).getDeserializeCall() and
10427
bth = badTypeNameHandling.asExpr()

0 commit comments

Comments
 (0)