Skip to content

Commit 1f79f38

Browse files
committed
Breaking down some logic into primitives, remove dead comment
1 parent 5231a1f commit 1f79f38

File tree

1 file changed

+29
-18
lines changed

1 file changed

+29
-18
lines changed

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

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,26 @@ private module GetFullPathToQualifierTaintTrackingConfiguration implements DataF
5151
}
5252
}
5353

54+
class ZipArchiveEntryClass extends Class{
55+
ZipArchiveEntryClass(){
56+
this.hasFullyQualifiedName("System.IO.Compression", "ZipArchiveEntry")
57+
}
58+
}
59+
60+
/**
61+
* The `FullName` property of `System.IO.Compression.ZipArchiveEntry`.
62+
*/
63+
class ZipArchiveEntryFullNameAccess extends Property{
64+
ZipArchiveEntryFullNameAccess(){
65+
this.getDeclaringType() instanceof ZipArchiveEntryClass and
66+
this.getName() = "FullName"
67+
}
68+
}
69+
5470
/** An access to the `FullName` property of a `ZipArchiveEntry`. */
5571
class ArchiveFullNameSource extends Source {
5672
ArchiveFullNameSource() {
57-
exists(PropertyAccess pa | this.asExpr() = pa |
58-
pa.getTarget().getDeclaringType().hasFullyQualifiedName("System.IO.Compression", "ZipArchiveEntry") and
59-
pa.getTarget().getName() = "FullName"
60-
)
73+
exists(ZipArchiveEntryFullNameAccess pa | pa.getAnAccess() = this.asExpr())
6174
}
6275
}
6376

@@ -125,10 +138,12 @@ private predicate safeCombineGetFullPathSequence(MethodCallGetFullPath mcGetFull
125138
class RootSanitizerMethodCall extends SanitizerMethodCall {
126139
RootSanitizerMethodCall() {
127140
exists(MethodSystemStringStartsWith sm | this.getTarget() = sm) and
128-
exists(Expr q, AbstractValue v |
141+
exists(Expr q, MethodCallGetFullPath mcGetFullPath |
129142
this.getQualifier() = q and
130-
v.(AbstractValues::BooleanValue).getValue() = true and
131-
exists(MethodCallGetFullPath mcGetFullPath | safeCombineGetFullPathSequence(mcGetFullPath, q))
143+
// JB1: Try and detect existentials with non-interelated variables
144+
// , AbstractValue v
145+
// v.(AbstractValues::BooleanValue).getValue() = true and
146+
safeCombineGetFullPathSequence(mcGetFullPath, q)
132147
)
133148
}
134149

@@ -179,9 +194,12 @@ private module SanitizedGuardTaintTrackingConfiguration implements DataFlow::Con
179194
}
180195

181196
predicate isSink(DataFlow::Node sink) {
182-
exists(RootSanitizerMethodCall smc |
183-
smc.getAnArgument() = sink.asExpr() or
184-
smc.getQualifier() = sink.asExpr()
197+
exists(RootSanitizerMethodCall smc, Expr e |
198+
e = sink.asExpr() and
199+
e = [
200+
smc.getAnArgument(),
201+
smc.getQualifier()
202+
]
185203
)
186204
}
187205
}
@@ -199,19 +217,12 @@ abstract private class AbstractWrapperSanitizerMethod extends AbstractSanitizerM
199217

200218
AbstractWrapperSanitizerMethod() {
201219
this.getReturnType() instanceof BoolType and
202-
this.getAParameter() = paramFilename
220+
paramFilename = this.getAParameter()
203221
}
204222

205223
Parameter paramFilePath() { result = paramFilename }
206224
}
207225

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-
215226
/**
216227
* A DirectWrapperSantizierMethod is a Method where
217228
* The function can /only/ returns true when passes through the RootSanitizerGuard

0 commit comments

Comments
 (0)