Skip to content

Commit 1601846

Browse files
committed
Add exclusion to the ZipSlip query to avoid FPs
1 parent 0065e6e commit 1601846

File tree

1 file changed

+22
-1
lines changed

1 file changed

+22
-1
lines changed

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import java
44
import semmle.code.java.dataflow.TaintTracking
55
import semmle.code.java.security.PathSanitizer
66
private import semmle.code.java.dataflow.ExternalFlow
7+
private import semmle.code.java.security.PathCreation
78

89
/**
910
* A method that returns the name of an archive entry.
@@ -40,5 +41,25 @@ module ZipSlipFlow = TaintTracking::Global<ZipSlipConfig>;
4041
* A sink that represents a file creation, such as a file write, copy or move operation.
4142
*/
4243
private class FileCreationSink extends DataFlow::Node {
43-
FileCreationSink() { sinkNode(this, "path-injection") }
44+
FileCreationSink() {
45+
sinkNode(this, "path-injection") and
46+
not isPathCreation(this)
47+
}
48+
}
49+
50+
/**
51+
* Holds if `sink` is a path creation node that doesn't imply a read/write filesystem operation.
52+
* This is to avoid creating new spurious alerts, since `PathCreation` sinks weren't
53+
* previosuly part of this query.
54+
*/
55+
private predicate isPathCreation(DataFlow::Node sink) {
56+
exists(PathCreation pc |
57+
pc.getAnInput() = sink.asExpr() and
58+
// exclude actual read/write operations included in `PathCreation`
59+
not pc.(Call)
60+
.getCallee()
61+
.getDeclaringType()
62+
.hasQualifiedName("java.io",
63+
["FileInputStream", "FileOutputStream", "FileReader", "FileWriter"])
64+
)
4465
}

0 commit comments

Comments
 (0)