Skip to content

Fixes Arbitrary file access during archive extraction djl_api #3737

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Jul 19, 2025

String entryName = entry.getName();
validateArchiveEntry(entry.getName(), dest);
set.add(entryName);
Path file = dest.resolve(entryName).toAbsolutePath();

Extracting files from a malicious zip file, or similar type of archive, is at risk of directory traversal attacks if filenames from the archive are not properly validated. Zip archives contain archive entries representing each file in the archive. These entries include a file path for the entry, but these file paths are not restricted and may contain unexpected special elements such as the directory traversal element (..). If these file paths are used to create a filesystem path, then a file operation may happen in an unexpected location. This can result in sensitive information being revealed or deleted, or an attacker being able to influence behavior by modifying unexpected files.

Fix the zipslip vulnerability, we must ensure that the output path constructed from the archive entry name resolves to a location that is strictly within the destination directory. The best way is to resolve the entry name to a path, canonicalize (or normalize) it, and check that it starts with the canonical destination directory path. If not, we should throw an exception and abort extraction of that entry.

fix should be applied within the unzip method in api/src/main/java/ai/djl/util/ZipUtils.java, specifically after the entry's path is resolved, but before any filesystem operation (directory creation, file write) occurs. We should use Path.normalize() and compare with Path.startsWith(), as recommended. also ensure that the validateArchiveEntry method (if its logic is not shown or insufficient) is replaced or augmented with the proper check inline.

Required changes:

  • Add a check after resolving file (the output path) to ensure that file.normalize().startsWith(dest.normalize()) (or better, canonical paths).
  • If the check fails, throw an IOException with a suitable message.
  • If possible, avoid changing the existing functionality except for adding the validation.

The vulnerability improper path validation during zip extraction. critical points in ZipUtil.java where file paths were constructed using zip entry names without checking for directory traversal sequences ('../'). The patched version added canonical path checks to prevent escaping the target directory. The Unpacker.process method handles entry extraction and was missing these checks in vulnerable versions, making it the primary vulnerable function.

References

Zip Slip Vulnerability
Path Traversal
CWE-22

@odaysec odaysec requested review from zachgk and a team as code owners July 19, 2025 15:33
@@ -53,9 +53,13 @@ public static void unzip(InputStream is, Path dest) throws IOException {
Set<String> set = new HashSet<>();
while ((entry = zis.getNextEntry()) != null) {
String entryName = entry.getName();
validateArchiveEntry(entry.getName(), dest);
// Remove or augment validateArchiveEntry, perform canonical path check
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateArchiveEntry() pretty much doing the same thing, why this is necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants