(CodeQL) Fixed finding: "Arbitrary file access during archive extraction ("Zip Slip") "

[pixeebot]

Remediation

This change fixes "Arbitrary file access during archive extraction ("Zip Slip")
" (id = zipslip) identified by CodeQL.

Details

This change fixes instances of ZipInputStream to protect against malicious entries that attempt to escape their "file root" and overwrite other files on the running filesystem.

Normally, when you're using ZipInputStream it's because you're processing zip files. That code might look like this:

File file = new File(unzipTargetDirectory, zipEntry.getName()); // use file name from zip entry
InputStream is = zip.getInputStream(zipEntry); // get the contents of the zip entry
IOUtils.copy(is, new FileOutputStream(file)); // write the contents to the provided file name

This looks fine when it encounters a normal zip entry within a zip file, looking something like this pseudo-data:

path: data/names.txt
contents: Zeus\nHelen\nLeda...

However, there's nothing to prevent an attacker from sending an evil entry in the zip that looks more like this:

path: ../../../../../etc/passwd
contents: root::0:0:root:/:/bin/sh

Yes, in the above code, which looks like every piece of zip-processing code you can find on the Internet, attackers could overwrite any files to which the application has access. This rule replaces the standard ZipInputStream with a hardened subclass which prevents access to entry paths that attempt to traverse directories above the current directory (which no normal zip file should ever do.) Our changes end up looking something like this:

+ import io.github.pixee.security.ZipSecurity;
  ...
- var zip = new ZipInputStream(is, StandardCharsets.UTF_8);
+ var zip = ZipSecurity.createHardenedInputStream(is, StandardCharsets.UTF_8);

More reading

• https://snyk.io/research/zip-slip-vulnerability
• https://github.com/snyk/zip-slip-vulnerability
• https://wiki.sei.cmu.edu/confluence/display/java/IDS04-J.+Safely+extract+files+from+ZipInputStream
• https://vulncat.fortify.com/en/detail?id=desc.dataflow.java.path_manipulation_zip_entry_overwrite

🧚🤖 Powered by Pixeebot

Feedback | Community | Docs | Codemod ID: codeql:java/zipslip

To fix the problem, we need to ensure that the output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations. This can be achieved by verifying that the normalized full path of the output file starts with a prefix that matches the destination directory. We will use java.nio.file.Path.normalize() and java.nio.file.Path.startsWith(..) for this purpose.

1. Modify the sanitizeHtmlFilesInZip method to include a check that ensures the normalized path of the extracted file starts with the temporary directory path.
2. Update the sanitizeZipFilename method to be more robust in handling directory traversal sequences.

To fix the problem, we need to ensure that the output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations. This can be achieved by verifying that the normalized full path of the output file starts with a prefix that matches the destination directory. We will use java.nio.file.Path.normalize() and java.nio.file.Path.startsWith(..) for this purpose.

1. Modify the unzipAndGetMainHtml method to validate the constructed file paths.
2. Ensure that the sanitizeZipFilename method is robust enough to handle all potential directory traversal sequences.