Skip to content

Commit 31a1f43

Browse files
committed
constrain TT for SanitizedGuardTaintTrackingconfiguration to be only sourced from methods where there is a rootsanitizerMethodCall wihtin it
1 parent f5197d7 commit 31a1f43

File tree

1 file changed

+19
-21
lines changed

1 file changed

+19
-21
lines changed

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

Lines changed: 19 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,21 +51,6 @@ private module GetFullPathToQualifierTaintTrackingConfiguration implements DataF
5151
}
5252
}
5353

54-
/**
55-
* DEPRECATED: Use `ZipSlip` instead.
56-
*
57-
* A taint tracking configuration for Zip Slip.
58-
*/
59-
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
60-
TaintTrackingConfiguration() { this = "ZipSlipTaintTracking" }
61-
62-
override predicate isSource(DataFlow::Node source) { source instanceof Source }
63-
64-
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
65-
66-
override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer }
67-
}
68-
6954
/** An access to the `FullName` property of a `ZipArchiveEntry`. */
7055
class ArchiveFullNameSource extends Source {
7156
ArchiveFullNameSource() {
@@ -134,8 +119,8 @@ private predicate safeCombineGetFullPathSequence(MethodCallGetFullPath mcGetFull
134119
* OR
135120
* There is a direct flow from Path.GetFullPath to qualifier of RootSanitizerMethodCall.
136121
*
137-
* It is not simply enough for the qualifier of String.StartsWith
138-
* to pass through Path.Combine without also passing through GetFullPath AFTER.
122+
* It is not simply enough for the qualifier of String.StartsWith
123+
* to pass through Path.Combine without also passing through GetFullPath AFTER.
139124
*/
140125
class RootSanitizerMethodCall extends SanitizerMethodCall {
141126
RootSanitizerMethodCall() {
@@ -186,7 +171,12 @@ module SanitizedGuardTT = TaintTracking::Global<SanitizedGuardTaintTrackingConfi
186171
* }
187172
*/
188173
private module SanitizedGuardTaintTrackingConfiguration implements DataFlow::ConfigSig {
189-
predicate isSource(DataFlow::Node source) { source instanceof DataFlow::ParameterNode }
174+
predicate isSource(DataFlow::Node source) {
175+
source instanceof DataFlow::ParameterNode and
176+
exists(RootSanitizerMethodCall smc |
177+
smc.getEnclosingCallable() = source.getEnclosingCallable()
178+
)
179+
}
190180

191181
predicate isSink(DataFlow::Node sink) {
192182
exists(RootSanitizerMethodCall smc |
@@ -215,6 +205,13 @@ abstract private class AbstractWrapperSanitizerMethod extends AbstractSanitizerM
215205
Parameter paramFilePath() { result = paramFilename }
216206
}
217207

208+
/* predicate aaaa(ZipSlipGuard g, DataFlow::ParameterNode source){
209+
exists(DataFlow::Node sink |
210+
sink = DataFlow::exprNode(g.getFilePathArgument()) and
211+
SanitizedGuardTT::flow(source, sink) and
212+
)
213+
} */
214+
218215
/**
219216
* A DirectWrapperSantizierMethod is a Method where
220217
* The function can /only/ returns true when passes through the RootSanitizerGuard
@@ -246,7 +243,7 @@ class DirectWrapperSantizierMethod extends AbstractWrapperSanitizerMethod {
246243
ret.getExpr().(BoolLiteral).getBoolValue() = false
247244
or
248245
// It passes the guard, contraining the function argument to the Guard argument.
249-
exists(ZipSlipGuard g, DataFlow::Node source, DataFlow::Node sink |
246+
exists(ZipSlipGuard g, DataFlow::ParameterNode source, DataFlow::Node sink |
250247
g.getEnclosingCallable() = this and
251248
source = DataFlow::parameterNode(paramFilename) and
252249
sink = DataFlow::exprNode(g.getFilePathArgument()) and
@@ -453,8 +450,8 @@ private module ZipSlipConfig implements DataFlow::ConfigSig {
453450
*/
454451
module ZipSlip = TaintTracking::Global<ZipSlipConfig>;
455452

456-
final class ZipSlipTaintTrackingConfiguration extends TaintTracking::Configuration {
457-
ZipSlipTaintTrackingConfiguration() { this = "ZipSlipTaintTrackingConfiguration" }
453+
deprecated class TaintTrackingConfiguration extends TaintTracking::Configuration {
454+
TaintTrackingConfiguration() { this = "ZipSlipTaintTrackingConfiguration" }
458455

459456
override predicate isSource(DataFlow::Node node) { node instanceof Source }
460457

@@ -463,6 +460,7 @@ final class ZipSlipTaintTrackingConfiguration extends TaintTracking::Configurati
463460
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
464461
super.isAdditionalTaintStep(pred, succ)
465462
or
463+
// If the sink is a method call, and the source is an argument to that method call
466464
exists(MethodCall mc | succ.asExpr() = mc and pred.asExpr() = mc.getAnArgument())
467465
}
468466

0 commit comments

Comments
 (0)