Additional check when unpacking archives. Contributed by Jason Lowe and Akira Ajisaka.

This commit is contained in:
Akira Ajisaka 2018-05-23 17:20:20 +09:00
parent 0f0d29a8d1
commit cedc28d4ab
No known key found for this signature in database
GPG Key ID: C1EDBB9CA400FD50
2 changed files with 47 additions and 9 deletions

View File

@ -620,15 +620,20 @@ public class FileUtil {
public static void unZip(File inFile, File unzipDir) throws IOException {
Enumeration<? extends ZipEntry> entries;
ZipFile zipFile = new ZipFile(inFile);
String targetDirPath = unzipDir.getCanonicalPath() + File.separator;
try {
entries = zipFile.entries();
while (entries.hasMoreElements()) {
ZipEntry entry = entries.nextElement();
if (!entry.isDirectory()) {
File file = new File(unzipDir, entry.getName());
if (!file.getCanonicalPath().startsWith(targetDirPath)) {
throw new IOException("expanding " + entry.getName()
+ " would create file outside of " + unzipDir);
}
InputStream in = zipFile.getInputStream(entry);
try {
File file = new File(unzipDir, entry.getName());
if (!file.getParentFile().mkdirs()) {
if (!file.getParentFile().isDirectory()) {
throw new IOException("Mkdirs failed to create " +
@ -738,6 +743,13 @@ public class FileUtil {
private static void unpackEntries(TarArchiveInputStream tis,
TarArchiveEntry entry, File outputDir) throws IOException {
String targetDirPath = outputDir.getCanonicalPath() + File.separator;
File outputFile = new File(outputDir, entry.getName());
if (!outputFile.getCanonicalPath().startsWith(targetDirPath)) {
throw new IOException("expanding " + entry.getName()
+ " would create entry outside of " + outputDir);
}
if (entry.isDirectory()) {
File subDir = new File(outputDir, entry.getName());
if (!subDir.mkdirs() && !subDir.isDirectory()) {
@ -752,7 +764,6 @@ public class FileUtil {
return;
}
File outputFile = new File(outputDir, entry.getName());
if (!outputFile.getParentFile().exists()) {
if (!outputFile.getParentFile().mkdirs()) {
throw new IOException("Mkdirs failed to create tar internal dir "

View File

@ -38,6 +38,7 @@ import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import java.net.UnknownHostException;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
@ -740,10 +741,8 @@ public class TestFileUtil {
@Test (timeout = 30000)
public void testUnZip() throws IOException {
// make sa simple zip
setupDirs();
// make a simple tar:
// make a simple zip
final File simpleZip = new File(del, FILE);
OutputStream os = new FileOutputStream(simpleZip);
ZipOutputStream tos = new ZipOutputStream(os);
@ -760,7 +759,7 @@ public class TestFileUtil {
tos.close();
}
// successfully untar it into an existing dir:
// successfully unzip it into an existing dir:
FileUtil.unZip(simpleZip, tmp);
// check result:
assertTrue(new File(tmp, "foo").exists());
@ -777,6 +776,34 @@ public class TestFileUtil {
}
}
@Test (timeout = 30000)
public void testUnZip2() throws IOException {
setupDirs();
// make a simple zip
final File simpleZip = new File(del, FILE);
OutputStream os = new FileOutputStream(simpleZip);
try (ZipOutputStream tos = new ZipOutputStream(os)) {
// Add an entry that contains invalid filename
ZipEntry ze = new ZipEntry("../foo");
byte[] data = "some-content".getBytes(StandardCharsets.UTF_8);
ze.setSize(data.length);
tos.putNextEntry(ze);
tos.write(data);
tos.closeEntry();
tos.flush();
tos.finish();
}
// Unzip it into an existing dir
try {
FileUtil.unZip(simpleZip, tmp);
Assert.fail("unZip should throw IOException.");
} catch (IOException e) {
GenericTestUtils.assertExceptionContains(
"would create file outside of", e);
}
}
@Test (timeout = 30000)
/*
* Test method copy(FileSystem srcFS, Path src, File dst, boolean deleteSource, Configuration conf)