Skip to content

Commit 1a1a3a3

Browse files
Jami CogswellJami Cogswell
authored andcommitted
Java: test perf
1 parent 523a1d0 commit 1a1a3a3

File tree

1 file changed

+21
-7
lines changed

1 file changed

+21
-7
lines changed

java/ql/lib/semmle/code/java/security/PathSanitizer.qll

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -354,8 +354,11 @@ private class FileGetNameSanitizer extends PathInjectionSanitizer {
354354
}
355355

356356
/** Holds if `expr` may be null. */
357-
private predicate maybeNull(Expr expr) {
358-
exists(DataFlow::Node src, DataFlow::Node sink |
357+
private predicate maybeNullArg(Expr expr) {
358+
exists(DataFlow::Node src, DataFlow::Node sink, ConstructorCall constrCall |
359+
constrCall.getConstructedType() instanceof TypeFile and
360+
constrCall.getNumArgument() = 2 and
361+
expr = constrCall.getArgument(0) and
359362
src.asExpr() = nullExpr() and
360363
sink.asExpr() = expr
361364
|
@@ -365,16 +368,27 @@ private predicate maybeNull(Expr expr) {
365368

366369
/** A taint-tracking configuration for reasoning about tainted nodes. */
367370
private module TaintedConfig implements DataFlow::ConfigSig {
368-
predicate isSource(DataFlow::Node source) { source instanceof ActiveThreatModelSource }
371+
predicate isSource(DataFlow::Node source) {
372+
source instanceof ActiveThreatModelSource
373+
or
374+
// ZipSlip uses ApiSourceNode instead of ActiveThreatModelSource
375+
source instanceof ApiSourceNode
376+
}
369377

370-
predicate isSink(DataFlow::Node sink) { any() }
378+
predicate isSink(DataFlow::Node sink) {
379+
exists(ConstructorCall constrCall |
380+
constrCall.getConstructedType() instanceof TypeFile and
381+
constrCall.getNumArgument() = 2 and
382+
sink.asExpr() = constrCall.getArgument(0)
383+
)
384+
}
371385
}
372386

373387
/** Tracks flow from any `ActiveThreatModelSource` to any node. */
374388
private module TaintedFlow = TaintTracking::Global<TaintedConfig>;
375389

376390
/** Holds if `expr is tainted by an `ActiveThreatModelSource`. */
377-
private predicate isTainted(Expr expr) { TaintedFlow::flowToExpr(expr) }
391+
private predicate isTaintedArg(Expr expr) { TaintedFlow::flowToExpr(expr) }
378392

379393
/** Holds if `g` is a guard that checks for `..` components. */
380394
private predicate pathTraversalGuard(Guard g, Expr e, boolean branch) {
@@ -394,8 +408,8 @@ private class FileConstructorSanitizer extends PathInjectionSanitizer {
394408
// Exclude cases where the parent argument is null since the
395409
// `java.io.File` documentation states that such cases are
396410
// treated as if invoking the single-argument `File` constructor.
397-
not maybeNull(constrCall.getArgument(0)) and
398-
not isTainted(constrCall.getArgument(0)) and
411+
not maybeNullArg(constrCall.getArgument(0)) and
412+
not isTaintedArg(constrCall.getArgument(0)) and
399413
arg = constrCall.getArgument(1) and
400414
(
401415
arg = DataFlow::BarrierGuard<pathTraversalGuard/3>::getABarrierNode().asExpr() or

0 commit comments

Comments
 (0)