Skip to content
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

Potential Zip Slip Vulnerability in restoreFromCheckpoint and decompress Methods #18697

Open
iris-sa-dev opened this issue Sep 24, 2024 · 0 comments
Labels
type-bug This issue is about a bug

Comments

@iris-sa-dev
Copy link

Alluxio Version:

2.9.4

Describe the bug

There is a potential security vulnerability related to Zip Slip (CWE-22) in the restoreFromCheckpoint method and the decompress method, which handle ZIP file extraction without sufficient validation of the file paths within the ZIP entries. Assuming that a malicious checkpoint file is passed to the restoreFromCheckpoint(CheckpointInputStream input) function:

public void restoreFromCheckpoint(CheckpointInputStream input) throws IOException {
    // ...
    if (input.getType() == CheckpointType.ROCKS_PARALLEL) {
      // ...
      String tmpZipFilePath = new File(tmpDirs.get(0), "alluxioRockStore-" + UUID.randomUUID()).getPath();
      try {
        try (FileOutputStream fos = new FileOutputStream(tmpZipFilePath)) {
          IOUtils.copy(input, fos); // input is stored to tmpZipFilePath
        }
        ParallelZipUtils.decompress(Paths.get(mDbPath), tmpZipFilePath, mParallelBackupPoolSize); // tmpZipFilePath is being decompressed
     // ...

This ZipFile is then read and unzipped, with each entry unzipped in the unzipEntry function:

private static void unzipEntry(ZipFile zipFile, ZipArchiveEntry entry, Path dirPath) throws Exception {
    File outputFile = new File(dirPath.toFile(), entry.getName()); // entry.getName() might contain malicious information such as "../", causing unwanted arbitrary directory traversal
    outputFile.getParentFile().mkdirs();
    if (entry.isDirectory()) {
      outputFile.mkdir();
    } else {
      try (InputStream inputStream = zipFile.getInputStream(entry);
           FileOutputStream fileOutputStream = new FileOutputStream(outputFile)) { // outputFile is directly used to open a new FileOutputStream, causing a file in an unwanted directory to be created

However, the entry.getName() function might return malicious entry name if the user crafted zip file contains malicious information. Specifically, the name could contain ../, which will allow traversal into unwanted paths. Indeed, the entry.getName() path is un-sanitized and directly passed to FileOutputStream.

To Reproduce

Create a malicious ZIP file with entries that contain path traversal sequences, such as ../ or /. Pass this ZIP file as a checkpoint to the restoreFromCheckpoint method. Observe that files are extracted outside of the expected directory.

Expected behavior

An exception should be thrown if an unwanted substring (such as ../) is found in a ZipArchiveEntry.

Urgency

This is a CWE-22 security vulnerability which will make the project vulnerable to whoever having access to the API for restoring from user supplied checkpoint.

@iris-sa-dev iris-sa-dev added the type-bug This issue is about a bug label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug This issue is about a bug
Projects
None yet
Development

No branches or pull requests

1 participant