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 2584a845d3e..4e699d2bb67 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 @@ -620,16 +620,21 @@ public static long getDU(File dir) { public static void unZip(File inFile, File unzipDir) throws IOException { Enumeration 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().mkdirs()) { if (!file.getParentFile().isDirectory()) { throw new IOException("Mkdirs failed to create " + file.getParentFile().toString()); @@ -738,6 +743,13 @@ private static void unTarUsingJava(File inFile, File untarDir, 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 @@ private static void unpackEntries(TarArchiveInputStream tis, 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 e156ec67b2d..bbda2e45a47 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 @@ -38,6 +38,7 @@ 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 void testCreateLocalTempFile() throws IOException { @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 void testUnZip() throws IOException { 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()); @@ -775,8 +774,36 @@ public void testUnZip() throws IOException { } 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)