Apache Commons Compress as solution to Zip Bomb

Question

Java Code is implemented to uncompress zip file using java.util.zip library. Sonarqube reports Security Hotspots vulnerability as prone to "Zip Bomb" security issue with message "Make sure that expanding this archive file is safe here" in the line "ZipEntry entry = zipIn.getNextEntry();".

As a solution, trying to use Apache Commons Compress version 1.21 library which handles Zip Bomb starting from version 1.17. For testing, downloaded a Zip Bomb Vulnerable zip file from here .

But this zip file gets uncompressed without any error/exception. What is wrong with this code mentioned under heading "Implementation using Apache Commons Compress Library"?

<dependency>
 <groupId>org.apache.commons</groupId>
 <artifactId>commons-compress</artifactId>
 <version>1.21</version>
</dependency>

Zip Bomb Vulnerable code

private void unzipNormal(String zipFilePath, String destDirectory) {
    try {
        File destDir = new File(destDirectory);
        if(!destDir.exists()) {
            destDir.mkdir();
        }

        try(ZipInputStream zipIn = new ZipInputStream(new FileInputStream(zipFilePath))) {
            ZipEntry entry = zipIn.getNextEntry();
            while(entry != null) {
                String filePath = destDirectory + File.separator + entry.getName();
                if(!entry.isDirectory()) {
                    extractFile(zipIn, filePath);
                } else {
                    File dir = new File(filePath);
                    dir.mkdir();
                }
                zipIn.closeEntry();
                entry = zipIn.getNextEntry();
            }
            zipIn.close();
        }

    } catch (Exception ex) {
        ex.printStackTrace();
    }
}

private static void extractFile(ZipInputStream zipIn, String filePath) throws IOException {
    try(BufferedOutputStream bos = new BufferedOutputStream(new FileOutputStream(filePath))) {
        byte[] bytesIn = new byte[4096];
        int read = 0;
        while((read = zipIn.read(bytesIn)) != -1) {
            bos.write(bytesIn, 0, read);
        }
        bos.close();
    } catch (Exception ex) {
        ex.printStackTrace();
        throw ex;
    }
}

Implementation using Apache Commons Compress Library

private void unzip(String srcZipFile, String destFolder) throws IOException {

        Path filePath = Paths.get(srcZipFile);

        try(InputStream inputStream = Files.newInputStream(filePath);
            ZipArchiveInputStream i = new ZipArchiveInputStream(inputStream)
        ) {
            System.out.println("Begin..");
            ArchiveEntry entry = null;
            while((entry = i.getNextEntry()) != null) {
                if(!i.canReadEntryData(entry)) {
                    System.out.println("Continue..");
                    continue;
                }

                Path path = Paths.get(destFolder, entry.getName());
                File f = path.toFile();
                if(entry.isDirectory()) {
                    if (!f.isDirectory() && !f.mkdirs()) {
                        throw new IOException("failed to create directory " + f);
                    }
                } else {
                    File parent = f.getParentFile();
                    if(!parent.isDirectory() && !parent.mkdirs()) {
                        throw new IOException("failed to create directory " + parent);
                    }
                    try (OutputStream o = Files.newOutputStream(f.toPath())) {
                        IOUtils.copy(i, o);
                    }
                }

            }
        } catch (Exception ex) {
            ex.printStackTrace();
        }
}

Answer 1

Unfortunately Apache Commons Compress itself does not protect against zip bombs. The library only exposes information (via InputStreamStatistics), and this can be used to manually implement zip bomb protection.

It'd be nice if protection was built-in, because I don't want to roll my own implementation. The documentation is not very clear and there's very few examples.

I've implemented zip bomb protection in Kotlin/JVM, which I hope is of some help.

Key points are:

• As mentioned above, InputStreamStatistics can be used to see how much uncompressed data has been extracted, and how much compressed data there was. Both are measured in bytes.
• The statistics are returned by ZipFile#getInputStream (The function returns InputStream, but the Javadoc says it will always implement InputStreamStatistics).
• InputStreamStatistics is specific per ZipArchiveEntry, not for the entire zip. IMPORTANT The statistics are only be updated after the entry's InputStream has been transferred.
• I don't want failures during unzipping to result in partial files left on the disk. So, I loop over the ZipFile twice.
  • The first loop does validation. It does not transfer file data to disk. To correctly measure the compressed/uncompressed bytes the entry is transferred to OutputStream#nullOutputStream (which requires Java 11, but it's easy to copy and paste the actual implementation for lower Java versions.)
  • The second loop actually transfers the files.
• I picked an arbitrary maximum uncompresssed-to-compressed ratio of 100. (E.g. a 100 byte entry that gets unpacked to 10MB would fail validation.) Another ratio might be more suitable.
• Another vulnerability to be aware of is Zip Slip, so let's protect against that while we're here.
• The Apache Compress docs recommend against using ZipArchiveInputStream (because ZipArchiveInputStream doesn't read a zip's 'central directory' it may misread the content.) Instead, I used the recommend ZipFile.

import java.io.OutputStream
import java.nio.file.Path
import kotlin.io.path.*
import org.apache.commons.compress.archivers.zip.ZipArchiveEntry
import org.apache.commons.compress.archivers.zip.ZipFile
import org.apache.commons.compress.utils.InputStreamStatistics

/**
 * Unzip [file] as a `.zip` file into [destinationDir].
 *
 * Protects against zip bombs by verifying the ratio of uncompresssed-to-compressed data does not exceed 100.
 */
internal fun unzip(
  file: Path,
  destinationDir: Path,
  /** Allowable maximum compression ratio. */
  maxCompressionRatio: Double = 100.0,
) {
  require(file.isRegularFile()) { "file must be a regular file." }
  require(destinationDir.isDirectory()) { "destinationDir must be a directory." }
  require(destinationDir.isAbsolute) { "destinationDir must be absolute." }

  /** ZipBomb validation on a specific entry. */
  fun InputStreamStatistics.validateCompression() {
    if (compressedCount < 1024 || uncompressedCount < 1024) {
      // not enough data to check yet, and avoid divide-by-zero
      return
    }
    val currentRatio = uncompressedCount.toDouble() / compressedCount.toDouble()
    require(currentRatio < maxCompressionRatio) {
      "Compression ratio on entry exceeded maximum ratio $maxCompressionRatio. ${file.invariantSeparatorsPathString}"
    }
  }

  /** Get the destination inside [destinationDir] for each zip entry.  */
  fun ZipArchiveEntry.destinationPath(): Path =
    destinationDir.resolve(name)

  /** ZipSlip validation - https://security.snyk.io/research/zip-slip-vulnerability */
  fun ZipArchiveEntry.validateDestinationPath() {
    val destinationPath = destinationPath()
    val canonicalDestinationFile = destinationPath.normalize().absolute()
    require(canonicalDestinationFile.startsWith(destinationDir)) {
      "Entry is outside of the target dir: $name"
    }
  }

  ZipFile.builder()
    .setPath(file)
    .get()
    .use { zipFile: ZipFile ->

      // STEP 1: validate each entry
      zipFile.entries.asIterator().forEachRemaining { entry: ZipArchiveEntry ->
        // validate all files against ZipSlip
        entry.validateDestinationPath()

        if (!entry.isDirectory) {
          zipFile.getInputStream(entry).use { archiveInput ->
            require(archiveInput is InputStreamStatistics) {
              "archiveInput must implement InputStreamStatistics, as per ZipFile.getInputStream() Javadoc."
            }
            archiveInput.transferTo(OutputStream.nullOutputStream())
            archiveInput.validateCompression()
          }
        }
      }

      // STEP 2: copy the contents to the destination
      zipFile.entries.asIterator().forEachRemaining { entry: ZipArchiveEntry ->
        val destinationPath = entry.destinationPath()
        when {
          entry.isDirectory -> destinationPath.createDirectories()
          else              -> {
            destinationPath.outputStream().use { output ->
              zipFile.getInputStream(entry).use { input ->
                input.transferTo(output)
              }
            }
          }
        }
    }
  }
}

I've tested it successfully against the 5.5GB and 281TB example zips from this website: https://bamsoftware.com/hacks/zipbomb/