Skip to content

fix Arbitrary file access during archive extraction ("Zip Slip") FileUtils() #230

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: main
Choose a base branch
from

Conversation

odaysec
Copy link

@odaysec odaysec commented Apr 19, 2025

Paths.get(tarEntry.getName()).normalize()).toFile();

fix the issue need to ensure that the extracted file paths are validated to prevent directory traversal attacks. Specifically:

  1. Normalize the path of the extracted file using Path.normalize() or File.getCanonicalFile().
  2. Verify that the normalized path starts with the intended base directory (AstraCliUtils.ASTRA_HOME) using Path.startsWith() or equivalent.
  3. If the path is invalid (i.e., it does not start with the base directory), throw an exception or skip the entry.

The changes will be made in the extractTarArchiveInAstraCliHome method. Specifically:

  • After constructing the outputFile path, validate that it starts with the intended base directory.
  • If the validation fails, log a warning and skip the entry.

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.

if a zip file contains a file entry ..\astra-cli-file, and the zip file is extracted to the directory c:\output, then naively combining the paths would result in an output file path of c:\output\..\astra-cli-file, which would cause the file to be written to c:\astra-file.

Recommendation

Ensure that output paths constructed from zip archive entries are validated to prevent writing files to unexpected locations. The recommended way of writing an output file from a zip archive entry is to verify that the normalized full path of the output file starts with a prefix that matches the destination directory. Path normalization can be done with either java.io.File.getCanonicalFile() or java.nio.file.Path.normalize(). Prefix checking can be done with String.startsWith(..), but it is better to use java.nio.file.Path.startsWith(..), as the latter works on complete path segments.

POC

a file path taken from a zip archive item entry is combined with a destination directory. The result is used as the destination file path without verifying that the result is within the destination directory. If provided with a zip file containing an archive path like ..\datastax-file, then this file would be written outside the destination directory.

void writeZipEntry(ZipEntry entry, File destinationDir) {
    File file = new File(destinationDir, entry.getName());
    FileOutputStream fos = new FileOutputStream(file); // BAD
    // ... write entry to fos ...
}

To fix this vulnerability, we need to verify that the normalized file still has destinationDir as its prefix, and throw an exception if this is not the case.

void writeZipEntry(ZipEntry entry, File destinationDir) {
    File file = new File(destinationDir, entry.getName());
    if (!file.toPath().normalize().startsWith(destinationDir.toPath()))
        throw new Exception("Bad zip entry");
    FileOutputStream fos = new FileOutputStream(file); // OK
    // ... write entry to fos ...
}

References

Zip Slip Vulnerability
Path Traversal
CWE-22

@msmygit msmygit requested review from clun and Copilot June 23, 2025 15:40
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes a Zip Slip vulnerability by validating the file paths during archive extraction in the Astra CLI.

  • Validates the normalized extracted file path against the base directory.
  • Logs a warning and skips any archive entry failing the check.

File outputFile = Paths.get(AstraCliUtils.ASTRA_HOME + File.separator +
Paths.get(tarEntry.getName()).normalize()).toFile();
Path outputPath = Paths.get(AstraCliUtils.ASTRA_HOME).resolve(Paths.get(tarEntry.getName()).normalize());
if (!outputPath.normalize().startsWith(Paths.get(AstraCliUtils.ASTRA_HOME))) {
Copy link
Preview

Copilot AI Jun 23, 2025

Choose a reason for hiding this comment

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

Consider using outputPath.toRealPath() instead of normalize() to account for potential symbolic links, ensuring the check against AstraCliUtils.ASTRA_HOME fully prevents directory traversal vulnerabilities.

Copilot uses AI. Check for mistakes.

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.

1 participant