Alluxio / alluxio

Alluxio, data orchestration for analytics and machine learning in the cloud
https://www.alluxio.io
Apache License 2.0
6.87k stars 2.94k forks source link

Potential Zip Slip Vulnerability in `restoreFromCheckpoint` and `decompress` Methods #18697

Open iris-sa-dev opened 1 month ago

iris-sa-dev commented 1 month ago

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.