Skip to content

Commit 8f6411d

Browse files
author
edvraa
committed
Simpify with exists
1 parent 0590522 commit 8f6411d

File tree

1 file changed

+97
-54
lines changed

1 file changed

+97
-54
lines changed

csharp/ql/src/semmle/code/csharp/security/dataflow/UnsafeDeserialization.qll

Lines changed: 97 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ module UnsafeDeserialization {
4848
class RemoteSource extends Source {
4949
RemoteSource() { this instanceof RemoteFlowSource }
5050
}
51-
51+
5252
class LocalSource extends Source {
5353
LocalSource() { this instanceof LocalFlowSource }
5454
}
@@ -145,14 +145,14 @@ module UnsafeDeserialization {
145145
private predicate isBinaryFormatterCall(MethodCall mc, Method m) {
146146
m = mc.getTarget() and
147147
(
148-
m instanceof BinaryFormatterDeserializeMethod and
149-
not mc.getArgument(0).hasValue()
150-
or
151-
m instanceof BinaryFormatterUnsafeDeserializeMethod and
152-
not mc.getArgument(0).hasValue()
153-
or
154-
m instanceof BinaryFormatterUnsafeDeserializeMethodResponseMethod and
155-
not mc.getArgument(0).hasValue()
148+
not mc.getArgument(0).hasValue() and
149+
(
150+
m instanceof BinaryFormatterDeserializeMethod
151+
or
152+
m instanceof BinaryFormatterUnsafeDeserializeMethod
153+
or
154+
m instanceof BinaryFormatterUnsafeDeserializeMethodResponseMethod
155+
)
156156
)
157157
}
158158

@@ -207,10 +207,11 @@ module UnsafeDeserialization {
207207
private predicate isNetDataContractSerializerCall(MethodCall mc, Method m) {
208208
m = mc.getTarget() and
209209
(
210-
m instanceof NetDataContractSerializerDeserializeMethod and
211-
not mc.getArgument(0).hasValue()
212-
or
213-
m instanceof NetDataContractSerializerReadObjectMethod and
210+
(
211+
m instanceof NetDataContractSerializerDeserializeMethod
212+
or
213+
m instanceof NetDataContractSerializerReadObjectMethod
214+
) and
214215
not mc.getArgument(0).hasValue()
215216
)
216217
}
@@ -250,10 +251,15 @@ module UnsafeDeserialization {
250251
}
251252

252253
override predicate isSource(DataFlow::Node source) {
253-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
254-
DataContractJsonSerializerClass and
255-
source.asExpr().(ObjectCreation).getTarget().getNumberOfParameters() > 0 and
256-
source.asExpr().(ObjectCreation).getArgument(0) instanceof TypeofExpr
254+
exists(ObjectCreation oc |
255+
oc = source.asExpr() and
256+
exists(Constructor c |
257+
c = oc.getTarget() and
258+
c.getDeclaringType() instanceof DataContractJsonSerializerClass and
259+
c.getNumberOfParameters() > 0 and
260+
oc.getArgument(0) instanceof TypeofExpr
261+
)
262+
)
257263
}
258264

259265
override predicate isSink(DataFlow::Node sink) {
@@ -268,10 +274,11 @@ module UnsafeDeserialization {
268274
private predicate isJavaScriptSerializerCall(MethodCall mc, Method m) {
269275
m = mc.getTarget() and
270276
(
271-
m instanceof JavaScriptSerializerClassDeserializeMethod and
272-
not mc.getArgument(0).hasValue()
273-
or
274-
m instanceof JavaScriptSerializerClassDeserializeObjectMethod and
277+
(
278+
m instanceof JavaScriptSerializerClassDeserializeMethod
279+
or
280+
m instanceof JavaScriptSerializerClassDeserializeObjectMethod
281+
) and
275282
not mc.getArgument(0).hasValue()
276283
)
277284
}
@@ -293,9 +300,14 @@ module UnsafeDeserialization {
293300
}
294301

295302
override predicate isSource(DataFlow::Node source) {
296-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
297-
JavaScriptSerializerClass and
298-
source.asExpr().(ObjectCreation).getTarget().getNumberOfParameters() = 0
303+
exists(ObjectCreation oc |
304+
oc = source.asExpr() and
305+
exists(Constructor c |
306+
c = oc.getTarget() and
307+
c.getDeclaringType() instanceof JavaScriptSerializerClass and
308+
c.getNumberOfParameters() = 0
309+
)
310+
)
299311
}
300312

301313
override predicate isSink(DataFlow::Node sink) {
@@ -331,13 +343,16 @@ module UnsafeDeserialization {
331343
}
332344

333345
override predicate isSource(DataFlow::Node source) {
334-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType().getABaseType+() instanceof
335-
XmlObjectSerializerClass and
336-
not (
337-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
338-
DataContractSerializerClass or
339-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
340-
NetDataContractSerializerClass
346+
exists(ObjectCreation oc |
347+
oc = source.asExpr() and
348+
exists(ValueOrRefType declaringType |
349+
declaringType = oc.getTarget().getDeclaringType() and
350+
declaringType.getABaseType+() instanceof XmlObjectSerializerClass and
351+
not (
352+
declaringType instanceof DataContractSerializerClass or
353+
declaringType instanceof NetDataContractSerializerClass
354+
)
355+
)
341356
)
342357
}
343358

@@ -373,9 +388,15 @@ module UnsafeDeserialization {
373388
}
374389

375390
override predicate isSource(DataFlow::Node source) {
376-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof XmlSerializerClass and
377-
source.asExpr().(ObjectCreation).getTarget().getNumberOfParameters() > 0 and
378-
source.asExpr().(ObjectCreation).getArgument(0) instanceof TypeofExpr
391+
exists(ObjectCreation oc |
392+
oc = source.asExpr() and
393+
exists(Constructor c |
394+
c = oc.getTarget() and
395+
c.getDeclaringType() instanceof XmlSerializerClass and
396+
c.getNumberOfParameters() > 0 and
397+
oc.getArgument(0) instanceof TypeofExpr
398+
)
399+
)
379400
}
380401

381402
override predicate isSink(DataFlow::Node sink) {
@@ -414,10 +435,15 @@ module UnsafeDeserialization {
414435
}
415436

416437
override predicate isSource(DataFlow::Node source) {
417-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
418-
DataContractSerializerClass and
419-
source.asExpr().(ObjectCreation).getTarget().getNumberOfParameters() > 0 and
420-
source.asExpr().(ObjectCreation).getArgument(0) instanceof TypeofExpr
438+
exists(ObjectCreation oc |
439+
oc = source.asExpr() and
440+
exists(Constructor c |
441+
c = oc.getTarget() and
442+
c.getDeclaringType() instanceof DataContractSerializerClass and
443+
c.getNumberOfParameters() > 0 and
444+
oc.getArgument(0) instanceof TypeofExpr
445+
)
446+
)
421447
}
422448

423449
override predicate isSink(DataFlow::Node sink) {
@@ -452,10 +478,15 @@ module UnsafeDeserialization {
452478
}
453479

454480
override predicate isSource(DataFlow::Node source) {
455-
source.asExpr().(ObjectCreation).getTarget().getDeclaringType() instanceof
456-
XmlMessageFormatterClass and
457-
source.asExpr().(ObjectCreation).getTarget().getNumberOfParameters() > 0 and
458-
source.asExpr().(ObjectCreation).getArgument(0) instanceof TypeofExpr
481+
exists(ObjectCreation oc |
482+
oc = source.asExpr() and
483+
exists(Constructor c |
484+
c = oc.getTarget() and
485+
c.getDeclaringType() instanceof XmlMessageFormatterClass and
486+
c.getNumberOfParameters() > 0 and
487+
oc.getArgument(0) instanceof TypeofExpr
488+
)
489+
)
459490
}
460491

461492
override predicate isSink(DataFlow::Node sink) {
@@ -635,9 +666,12 @@ module UnsafeDeserialization {
635666
or
636667
m instanceof ServiceStackTextJsonSerializerDeserializeFromStreamMethod
637668
) and
638-
not mc.getAnArgument().hasValue() and
639-
not mc.getAnArgument() instanceof TypeofExpr and
640-
this.asExpr() = mc.getAnArgument()
669+
exists(Expr arg |
670+
arg = mc.getAnArgument() and
671+
not arg.hasValue() and
672+
not arg instanceof TypeofExpr and
673+
this.asExpr() = arg
674+
)
641675
)
642676
}
643677
}
@@ -657,9 +691,12 @@ module UnsafeDeserialization {
657691
or
658692
m instanceof ServiceStackTextTypeSerializerDeserializeFromStreamMethod
659693
) and
660-
not mc.getAnArgument().hasValue() and
661-
not mc.getAnArgument() instanceof TypeofExpr and
662-
this.asExpr() = mc.getAnArgument()
694+
exists(Expr arg |
695+
arg = mc.getAnArgument() and
696+
not arg.hasValue() and
697+
not arg instanceof TypeofExpr and
698+
this.asExpr() = arg
699+
)
663700
)
664701
}
665702
}
@@ -678,9 +715,12 @@ module UnsafeDeserialization {
678715
or
679716
m instanceof ServiceStackTextCsvSerializerDeserializeFromStreamMethod
680717
) and
681-
not mc.getAnArgument().hasValue() and
682-
not mc.getAnArgument() instanceof TypeofExpr and
683-
this.asExpr() = mc.getAnArgument()
718+
exists(Expr arg |
719+
arg = mc.getAnArgument() and
720+
not arg.hasValue() and
721+
not arg instanceof TypeofExpr and
722+
this.asExpr() = arg
723+
)
684724
)
685725
}
686726
}
@@ -699,9 +739,12 @@ module UnsafeDeserialization {
699739
or
700740
m instanceof ServiceStackTextXmlSerializerDeserializeFromStreamMethod
701741
) and
702-
not mc.getAnArgument().hasValue() and
703-
not mc.getAnArgument() instanceof TypeofExpr and
704-
this.asExpr() = mc.getAnArgument()
742+
exists(Expr arg |
743+
arg = mc.getAnArgument() and
744+
not arg.hasValue() and
745+
not arg instanceof TypeofExpr and
746+
this.asExpr() = arg
747+
)
705748
)
706749
}
707750
}

0 commit comments

Comments
 (0)