-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Description
Description
The java/zipslip query (CWE-022) may report a false positive when archive entry names are used in non-extraction contexts, such as classpath resource lookup via ClassLoader.
In the reported case, the archive entry name is not used to construct a filesystem path for extraction or writing. Instead, it is only used for resource discovery. This does not appear to match the intended threat model of Zip Slip, which focuses on unsafe archive extraction leading to path traversal outside a target directory.
Root cause analysis
The current implementation of java/zipslip defines its sink as:
private class FileCreationSink extends DataFlow::Node {
FileCreationSink() { sinkNode(this, "path-injection") }
}This relies on the shared path-injection sink label from Models-as-Data (MaD), which includes a broad set of path-related APIs. These include:
- ClassLoader resource lookup APIs (for example,
getSystemResources,getResource,getResourceAsStream) - Read-only filesystem checks (for example,
File.exists,File.isDirectory) - File input streams
These sinks are appropriate for the general java/path-injection query. However, not all of them correspond to archive extraction or file write operations, which are central to Zip Slip vulnerabilities.
As a result, flows where archive entry names are used only for lookup or read-like operations may be reported as Zip Slip, even though no file extraction or write occurs.
Example
Discovered while scanning Apache Dubbo (branch 3.3):
private void scanJar(JarFile jar) {
Enumeration<JarEntry> entry = jar.entries();
while (entry.hasMoreElements()) {
JarEntry jarEntry = entry.nextElement();
String name = jarEntry.getName();
if (name.charAt(0) == '/') {
name = name.substring(1);
}
if (jarEntry.isDirectory()) {
continue;
}
if (matchedDubboClasses(name)) {
classNameCache.put(name, toClassName(name));
} else {
resourcePathCache.add(name);
}
}
}Later:
resources = ClassLoader.getSystemResources(fileName);Here, the archive entry name flows into a classpath lookup. No file is created, written, or extracted to the filesystem.
Expected behavior
The java/zipslip query should ideally focus on flows where:
- Archive entry names are used to construct filesystem paths, and
- Those paths are used in file creation, write, move, or extraction-related operations
Flows that only involve:
- ClassLoader-based resource lookup, or
- Read-only file access
do not appear to match the Zip Slip threat model.
Suggested improvement
To better align with the intended semantics of Zip Slip, the query could consider narrowing its sink definition. For example:
- Restrict sinks to APIs that create or write files, or otherwise materialize archive entries onto the filesystem
- Alternatively, introduce a more specific sink category for extraction-related path usage instead of reusing the full
path-injectionlabel
This could help reduce false positives without affecting the broader java/path-injection query.
Additional note
This report does not question the correctness of the path-injection modeling in MaD, which appears appropriate for general path traversal queries. The issue seems to arise specifically from how java/zipslip reuses that modeling.
I would be happy to submit a PR to fix :)