commit 45a1c680c276c4501402f7bc4cebcf85a6fbc7f5 Author: Akira Ajisaka Date: Wed May 23 17:21:46 2018 +0900 Additional check when unpacking archives. Contributed by Jason Lowe and Akira Ajisaka. 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 23fb9462449..00381fee278 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 @@ -587,16 +587,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()); @@ -705,6 +710,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()) { @@ -719,7 +731,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 41794b85314..7712535c669 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 @@ -25,8 +25,9 @@ import java.io.FileReader; import java.io.IOException; import java.io.OutputStream; -import java.net.URI; import java.io.PrintWriter; +import java.net.URI; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; @@ -40,6 +41,7 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.util.Shell; import org.apache.hadoop.util.StringUtils; import org.apache.tools.tar.TarEntry; @@ -708,10 +710,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); @@ -728,7 +728,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()); @@ -743,8 +743,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) commit eaa2b8035b584dfcf7c79a33484eb2dffd3fdb11 Author: Kihwal Lee Date: Tue May 29 14:47:55 2018 -0500 Additional check when unpacking archives. Contributed by Wilfred Spiegelenburg. diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java index 4b26b7611d6..a3b5b0bbd94 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/RunJar.java @@ -93,6 +93,7 @@ public static void unJar(File jarFile, File toDir, Pattern unpackRegex) throws IOException { JarFile jar = new JarFile(jarFile); try { + String targetDirPath = toDir.getCanonicalPath() + File.separator; Enumeration entries = jar.entries(); while (entries.hasMoreElements()) { final JarEntry entry = entries.nextElement(); @@ -102,6 +103,10 @@ public static void unJar(File jarFile, File toDir, Pattern unpackRegex) try { File file = new File(toDir, entry.getName()); ensureDirectory(file.getParentFile()); + if (!file.getCanonicalPath().startsWith(targetDirPath)) { + throw new IOException("expanding " + entry.getName() + + " would create file outside of " + toDir); + } OutputStream out = new FileOutputStream(file); try { IOUtils.copyBytes(in, out, 8192); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java index f592d0400a4..b2a65379eda 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestRunJar.java @@ -17,6 +17,7 @@ */ package org.apache.hadoop.util; +import static org.junit.Assert.fail; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -25,6 +26,8 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; +import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import java.util.regex.Pattern; import java.util.zip.ZipEntry; @@ -32,6 +35,7 @@ import junit.framework.TestCase; import org.apache.hadoop.fs.FileUtil; +import org.apache.hadoop.test.GenericTestUtils; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -169,4 +173,37 @@ private File makeClassLoaderTestJar(String... clsNames) throws IOException { return jarFile; } -} \ No newline at end of file + + @Test + public void testUnJar2() throws IOException { + // make a simple zip + File jarFile = new File(TEST_ROOT_DIR, TEST_JAR_NAME); + JarOutputStream jstream = + new JarOutputStream(new FileOutputStream(jarFile)); + JarEntry je = new JarEntry("META-INF/MANIFEST.MF"); + byte[] data = "Manifest-Version: 1.0\nCreated-By: 1.8.0_1 (Manual)" + .getBytes(StandardCharsets.UTF_8); + je.setSize(data.length); + jstream.putNextEntry(je); + jstream.write(data); + jstream.closeEntry(); + je = new JarEntry("../outside.path"); + data = "any data here".getBytes(StandardCharsets.UTF_8); + je.setSize(data.length); + jstream.putNextEntry(je); + jstream.write(data); + jstream.closeEntry(); + jstream.close(); + + File unjarDir = new File(TEST_ROOT_DIR, "unjar-path"); + + // Unjar everything + try { + RunJar.unJar(jarFile, unjarDir); + fail("unJar should throw IOException."); + } catch (IOException e) { + GenericTestUtils.assertExceptionContains( + "would create file outside of", e); + } + } +}