Additional check when unpacking archives. Contributed by Jason Lowe and Akira Ajisaka.
Project: http://git-wip-us.apache.org/repos/asf/hadoop/repo Commit: http://git-wip-us.apache.org/repos/asf/hadoop/commit/745f203e Tree: http://git-wip-us.apache.org/repos/asf/hadoop/tree/745f203e Diff: http://git-wip-us.apache.org/repos/asf/hadoop/diff/745f203e Branch: refs/heads/HDDS-48 Commit: 745f203e577bacb35b042206db94615141fa5e6f Parents: 1d2640b Author: Akira Ajisaka <aajis...@apache.org> Authored: Wed May 23 17:15:57 2018 +0900 Committer: Akira Ajisaka <aajis...@apache.org> Committed: Wed May 23 17:16:23 2018 +0900 ---------------------------------------------------------------------- .../java/org/apache/hadoop/fs/FileUtil.java | 18 ++++++++- .../java/org/apache/hadoop/fs/TestFileUtil.java | 40 +++++++++++++++++--- 2 files changed, 51 insertions(+), 7 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hadoop/blob/745f203e/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FileUtil.java ---------------------------------------------------------------------- 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 8743be5..5ef78f2 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 @@ -617,11 +617,16 @@ public class FileUtil { throws IOException { try (ZipInputStream zip = new ZipInputStream(inputStream)) { int numOfFailedLastModifiedSet = 0; + String targetDirPath = toDir.getCanonicalPath() + File.separator; for(ZipEntry entry = zip.getNextEntry(); entry != null; entry = zip.getNextEntry()) { if (!entry.isDirectory()) { File file = new File(toDir, entry.getName()); + if (!file.getCanonicalPath().startsWith(targetDirPath)) { + throw new IOException("expanding " + entry.getName() + + " would create file outside of " + toDir); + } File parent = file.getParentFile(); if (!parent.mkdirs() && !parent.isDirectory()) { @@ -656,12 +661,17 @@ public class FileUtil { try { entries = zipFile.entries(); + String targetDirPath = unzipDir.getCanonicalPath() + File.separator; while (entries.hasMoreElements()) { ZipEntry entry = entries.nextElement(); if (!entry.isDirectory()) { InputStream in = zipFile.getInputStream(entry); try { File file = new File(unzipDir, entry.getName()); + if (!file.getCanonicalPath().startsWith(targetDirPath)) { + throw new IOException("expanding " + entry.getName() + + " would create file outside of " + unzipDir); + } if (!file.getParentFile().mkdirs()) { if (!file.getParentFile().isDirectory()) { throw new IOException("Mkdirs failed to create " + @@ -944,6 +954,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()) { @@ -966,7 +983,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 " http://git-wip-us.apache.org/repos/asf/hadoop/blob/745f203e/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java ---------------------------------------------------------------------- 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 39f2f6b..7218a1b 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 @@ -23,6 +23,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -38,6 +39,7 @@ import java.net.URI; import java.net.URISyntaxException; import java.net.URL; import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystems; import java.nio.file.Files; import java.util.ArrayList; @@ -685,10 +687,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); @@ -705,7 +705,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()); @@ -720,8 +720,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); + 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) --------------------------------------------------------------------- To unsubscribe, e-mail: common-commits-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-commits-h...@hadoop.apache.org
