Skip to content

Commit b3de105

Browse files
committed
C#: Re-factor TypeNameTracking to use the new API.
1 parent ee7d15a commit b3de105

File tree

2 files changed

+61
-6
lines changed

2 files changed

+61
-6
lines changed

csharp/ql/lib/semmle/code/csharp/security/dataflow/UnsafeDeserializationQuery.qll

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@ class JsonConvertTrackingConfig extends TaintTracking::Configuration {
7575
}
7676

7777
/**
78+
* DEPRECATED: Use `TypeNameTracking` instead.
79+
*
7880
* Tracks unsafe `TypeNameHandling` setting to `JsonConvert` call
7981
*/
80-
class TypeNameTrackingConfig extends DataFlow::Configuration {
82+
deprecated class TypeNameTrackingConfig extends DataFlow::Configuration {
8183
TypeNameTrackingConfig() { this = "TypeNameTrackingConfig" }
8284

8385
override predicate isSource(DataFlow::Node source) {
@@ -127,6 +129,62 @@ class TypeNameTrackingConfig extends DataFlow::Configuration {
127129
}
128130
}
129131

132+
/**
133+
* Configuration module for tracking unsafe `TypeNameHandling` setting to `JsonConvert` calls.
134+
*/
135+
private module TypeNameTrackingConfig implements DataFlow::ConfigSig {
136+
predicate isSource(DataFlow::Node source) {
137+
(
138+
source.asExpr() instanceof MemberConstantAccess and
139+
source.getType() instanceof TypeNameHandlingEnum
140+
or
141+
source.asExpr() instanceof IntegerLiteral
142+
) and
143+
source.asExpr().hasValue() and
144+
not source.asExpr().getValue() = "0"
145+
}
146+
147+
predicate isSink(DataFlow::Node sink) {
148+
exists(MethodCall mc, Method m, Expr expr |
149+
m = mc.getTarget() and
150+
(
151+
not mc.getArgument(0).hasValue() and
152+
m instanceof NewtonsoftJsonConvertClassDeserializeObjectMethod
153+
) and
154+
expr = mc.getAnArgument() and
155+
sink.asExpr() = expr and
156+
expr.getType() instanceof JsonSerializerSettingsClass
157+
)
158+
}
159+
160+
predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
161+
node1.asExpr() instanceof IntegerLiteral and
162+
node2.asExpr().(CastExpr).getExpr() = node1.asExpr()
163+
or
164+
node1.getType() instanceof TypeNameHandlingEnum and
165+
exists(PropertyWrite pw, Property p, Assignment a |
166+
a.getLValue() = pw and
167+
pw.getProperty() = p and
168+
p.getDeclaringType() instanceof JsonSerializerSettingsClass and
169+
p.hasName("TypeNameHandling") and
170+
(
171+
node1.asExpr() = a.getRValue() and
172+
node2.asExpr() = pw.getQualifier()
173+
or
174+
exists(ObjectInitializer oi |
175+
node1.asExpr() = oi.getAMemberInitializer().getRValue() and
176+
node2.asExpr() = oi
177+
)
178+
)
179+
)
180+
}
181+
}
182+
183+
/**
184+
* Configuration module for tracking unsafe `TypeNameHandling` setting to `JsonConvert` calls.
185+
*/
186+
module TypeNameTracking = DataFlow::Global<TypeNameTrackingConfig>;
187+
130188
/**
131189
* User input to static method or constructor call deserialization flow tracking.
132190
*/

csharp/ql/src/Security Features/CWE-502/UnsafeDeserializationUntrustedInput.ql

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,9 @@ where
4848
)
4949
or
5050
// JsonConvert static method call, but with additional unsafe typename tracking
51-
exists(
52-
JsonConvertTrackingConfig taintTrackingJsonConvert, TypeNameTrackingConfig typenameTracking,
53-
DataFlow::Node settingsCallArg
54-
|
51+
exists(JsonConvertTrackingConfig taintTrackingJsonConvert, DataFlow::Node settingsCallArg |
5552
taintTrackingJsonConvert.hasFlowPath(userInput, deserializeCallArg) and
56-
typenameTracking.hasFlow(_, settingsCallArg) and
53+
TypeNameTracking::flow(_, settingsCallArg) and
5754
deserializeCallArg.getNode().asExpr().getParent() = settingsCallArg.asExpr().getParent()
5855
)
5956
select deserializeCallArg, userInput, deserializeCallArg, "$@ flows to unsafe deserializer.",

0 commit comments

Comments
 (0)