Permalink
Browse files

SONAR-10661 fix vulnerability in ZipUtils#unzip()

  • Loading branch information...
simonbrandhof authored and SonarTech committed May 4, 2018
1 parent 78cddc3 commit 08438a2c47112f2fce1e512f6c843c908abed4c7
@@ -26,6 +26,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Path;
import java.util.Enumeration;
import java.util.function.Predicate;
import java.util.zip.ZipEntry;
@@ -101,6 +102,8 @@ public static File unzip(InputStream stream, File toDir, Predicate<ZipEntry> fil

private static void unzipEntry(ZipEntry entry, ZipInputStream zipStream, File toDir) throws IOException {
File to = new File(toDir, entry.getName());
verifyInsideTargetDirectory(entry, to.toPath(), toDir.toPath());

if (entry.isDirectory()) {
throwExceptionIfDirectoryIsNotCreatable(to);
} else {
@@ -139,19 +142,23 @@ public static File unzip(File zip, File toDir, Predicate<ZipEntry> filter) throw
FileUtils.forceMkdir(toDir);
}

Path targetDirNormalizedPath = toDir.toPath().normalize();
ZipFile zipFile = new ZipFile(zip);
try {
Enumeration<? extends ZipEntry> entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
if (filter.test(entry)) {
File to = new File(toDir, entry.getName());
File target = new File(toDir, entry.getName());

verifyInsideTargetDirectory(entry, target.toPath(), targetDirNormalizedPath);

if (entry.isDirectory()) {
throwExceptionIfDirectoryIsNotCreatable(to);
throwExceptionIfDirectoryIsNotCreatable(target);
} else {
File parent = to.getParentFile();
File parent = target.getParentFile();
throwExceptionIfDirectoryIsNotCreatable(parent);
copy(zipFile, entry, to);
copy(zipFile, entry, target);
}
}
}
@@ -238,6 +245,13 @@ private static void doZipDir(File dir, ZipOutputStream out) throws IOException {
}
}

private static void verifyInsideTargetDirectory(ZipEntry entry, Path entryPath, Path targetDirPath) {
if (!entryPath.normalize().startsWith(targetDirPath.normalize())) {
// vulnerability - trying to create a file outside the target directory
throw new IllegalStateException("Unzipping an entry outside the target directory is not allowed: " + entry.getName());
}
}

/**
* @see #unzip(File, File, Predicate)
* @deprecated replaced by {@link Predicate<ZipEntry>} in 6.2.
@@ -20,26 +20,29 @@
package org.sonar.api.utils;

import com.google.common.collect.Iterators;
import java.net.URL;
import org.apache.commons.io.FileUtils;
import org.assertj.core.util.Files;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.io.FileInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.net.URL;
import java.util.Iterator;
import java.util.zip.ZipEntry;
import java.util.zip.ZipFile;
import org.apache.commons.io.FileUtils;
import org.assertj.core.util.Files;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TemporaryFolder;

import static org.assertj.core.api.Assertions.assertThat;

public class ZipUtilsTest {

@Rule
public TemporaryFolder temp = new TemporaryFolder();
@Rule
public ExpectedException expectedException = ExpectedException.none();

@Test
public void zip_directory() throws IOException {
@@ -106,6 +109,30 @@ public void unzipping_stream_extracts_subset_of_files() throws IOException {
assertThat(toDir.listFiles()).containsOnly(new File(toDir, "foo.txt"));
}

@Test
public void fail_if_unzipping_file_outside_target_directory() throws Exception {
File zip = new File(getClass().getResource("ZipUtilsTest/zip-slip.zip").toURI());
File toDir = temp.newFolder();

expectedException.expect(IllegalStateException.class);
expectedException.expectMessage("Unzipping an entry outside the target directory is not allowed: ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txt");

ZipUtils.unzip(zip, toDir);
}

@Test
public void fail_if_unzipping_stream_outside_target_directory() throws Exception {
File zip = new File(getClass().getResource("ZipUtilsTest/zip-slip.zip").toURI());
File toDir = temp.newFolder();

expectedException.expect(IllegalStateException.class);
expectedException.expectMessage("Unzipping an entry outside the target directory is not allowed: ../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../../tmp/evil.txt");

try (InputStream input = new FileInputStream(zip)) {
ZipUtils.unzip(input, toDir);
}
}

private URL urlToZip() {
return getClass().getResource("/org/sonar/api/utils/ZipUtilsTest/shouldUnzipFile.zip");
}

0 comments on commit 08438a2

Please sign in to comment.