diff options
author | Akira Ajisaka <aajisaka@apache.org> | 2018-05-23 17:19:18 +0900 |
---|---|---|
committer | Akira Ajisaka <aajisaka@apache.org> | 2018-05-23 17:19:18 +0900 |
commit | bd98d4e77cf9f7b2f4b1afb4d5e5bad0f6b2fde3 (patch) | |
tree | f5a84d80642318dd4b44f4a64fe2d915c8638c0a | |
parent | 5b3a6e36784ad49c39de41635a188f8b3f6c9b7d (diff) |
Additional check when unpacking archives. Contributed by Jason Lowe and Akira Ajisaka.
-rw-r--r-- | hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java | 15 | ||||
-rw-r--r-- | hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java | 39 |
2 files changed, 46 insertions, 8 deletions
diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java index 4d971aa815..580827f2e1 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java @@ -585,15 +585,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 " + @@ -703,6 +708,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()) { @@ -717,7 +729,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 " diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java index 0ad03fcbac..4651a403c1 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java @@ -37,6 +37,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; @@ -679,10 +680,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); @@ -699,7 +698,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()); @@ -714,8 +713,36 @@ public class TestFileUtil { } catch (IOException ioe) { // okay } - } - + } + + @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) |