CVE-2022-37865 ZipPacking allows overwriting arbitrary files
diff --git a/src/java/org/apache/ivy/core/pack/ZipPacking.java b/src/java/org/apache/ivy/core/pack/ZipPacking.java
index 18a010e..197463c 100644
--- a/src/java/org/apache/ivy/core/pack/ZipPacking.java
+++ b/src/java/org/apache/ivy/core/pack/ZipPacking.java
@@ -52,8 +52,15 @@
try (ZipInputStream zip = new ZipInputStream(packed)) {
ZipEntry entry = null;
while (((entry = zip.getNextEntry()) != null)) {
- File f = new File(dest, entry.getName());
- Message.verbose("\t\texpanding " + entry.getName() + " to " + f);
+ String entryName = entry.getName();
+ File f = FileUtil.resolveFile(dest, entryName);
+ if (!FileUtil.isLeadingPath(dest, f, true)) {
+ Message.verbose("\t\tskipping " + entryName + " as its target "
+ + f.getCanonicalPath()
+ + " is outside of " + dest.getCanonicalPath() + ".");
+ continue;
+ }
+ Message.verbose("\t\texpanding " + entryName + " to " + f);
// create intermediary directories - sometimes zip don't add them
File dirF = f.getParentFile();
diff --git a/src/java/org/apache/ivy/util/FileUtil.java b/src/java/org/apache/ivy/util/FileUtil.java
index e5e31d5..4d38741 100644
--- a/src/java/org/apache/ivy/util/FileUtil.java
+++ b/src/java/org/apache/ivy/util/FileUtil.java
@@ -611,6 +611,68 @@
}
/**
+ * Learn whether one path "leads" another.
+ *
+ * <p>This method uses {@link #normalize} under the covers and
+ * does not resolve symbolic links.</p>
+ *
+ * <p>If either path tries to go beyond the file system root
+ * (i.e. it contains more ".." segments than can be travelled up)
+ * the method will return false.</p>
+ *
+ * @param leading The leading path, must not be null, must be absolute.
+ * @param path The path to check, must not be null, must be absolute.
+ * @return true if path starts with leading; false otherwise.
+ * @since Ant 1.7
+ */
+ public static boolean isLeadingPath(File leading, File path) {
+ String l = normalize(leading.getAbsolutePath()).getAbsolutePath();
+ String p = normalize(path.getAbsolutePath()).getAbsolutePath();
+ if (l.equals(p)) {
+ return true;
+ }
+ // ensure that l ends with a /
+ // so we never think /foo was a parent directory of /foobar
+ if (!l.endsWith(File.separator)) {
+ l += File.separator;
+ }
+ // ensure "/foo/" is not considered a parent of "/foo/../../bar"
+ String up = File.separator + ".." + File.separator;
+ if (l.contains(up) || p.contains(up) || (p + File.separator).contains(up)) {
+ return false;
+ }
+ return p.startsWith(l);
+ }
+
+ /**
+ * Learn whether one path "leads" another.
+ *
+ * @param leading The leading path, must not be null, must be absolute.
+ * @param path The path to check, must not be null, must be absolute.
+ * @param resolveSymlinks whether symbolic links shall be resolved
+ * prior to comparing the paths.
+ * @return true if path starts with leading; false otherwise.
+ * @since Ant 1.9.13
+ * @throws IOException if resolveSymlinks is true and invoking
+ * getCanonicaPath on either argument throws an exception
+ */
+ public static boolean isLeadingPath(File leading, File path, boolean resolveSymlinks)
+ throws IOException {
+ if (!resolveSymlinks) {
+ return isLeadingPath(leading, path);
+ }
+ final File l = leading.getCanonicalFile();
+ File p = path.getCanonicalFile();
+ do {
+ if (l.equals(p)) {
+ return true;
+ }
+ p = p.getParentFile();
+ } while (p != null);
+ return false;
+ }
+
+ /**
* Get the length of the file, or the sum of the children lengths if it is a directory
*
* @param file File
diff --git a/test/java/org/apache/ivy/ant/FileUtilTest.java b/test/java/org/apache/ivy/ant/FileUtilTest.java
index ca96483..dd1131c 100644
--- a/test/java/org/apache/ivy/ant/FileUtilTest.java
+++ b/test/java/org/apache/ivy/ant/FileUtilTest.java
@@ -45,6 +45,7 @@
public class FileUtilTest {
private static boolean symlinkCapable = false;
+ private static final String PATH_SEP = System.getProperty("path.separator");
@BeforeClass
public static void beforeClass() {
@@ -151,4 +152,75 @@
Assert.assertTrue("Unexpected content in dest file " + destFile, Arrays.equals(fileContent, Files.readAllBytes(destFile)));
}
+ /**
+ * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=62502"
+ */
+ @Test
+ public void isLeadingPathCannotBeFooledByTooManyDoubleDots() {
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/../../bar")));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\..\\..\\bar")));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/../..")));
+ }
+
+ /**
+ * @see "https://bz.apache.org/bugzilla/show_bug.cgi?id=62502"
+ */
+ @Test
+ public void isLeadingPathCanonicalVersionCannotBeFooledByTooManyDoubleDots() throws IOException {
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/../../bar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\..\\..\\bar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/../.."), true));
+ }
+
+ @Test
+ public void isLeadingPathCanonicalVersionWorksAsExpectedOnUnix() throws IOException {
+ Assume.assumeFalse("Test doesn't run on DOS", PATH_SEP.equals(";"));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/baz/../bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/../foo/bar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/foobar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("/foo"), new File("/bar"), true));
+ }
+
+ @Test
+ public void isLeadingPathAndTrailingSlashesOnUnix() throws IOException {
+ Assume.assumeFalse("Test doesn't run on DOS", PATH_SEP.equals(";"));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/bar/"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/"), true));
+
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/bar"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/bar/"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo/"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo/"), new File("/foo"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("/foo"), new File("/foo/"), false));
+ }
+
+ @Test
+ public void isLeadingPathCanonicalVersionWorksAsExpectedOnDos() throws IOException {
+ Assume.assumeTrue("Test only runs on DOS", PATH_SEP.equals(";"));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\baz\\..\\bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("C:\\foo"), new File("C:\\foo\\..\\foo\\bar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("C:\\foo"), new File("C:\\foobar"), true));
+ Assert.assertFalse(FileUtil.isLeadingPath(new File("C:\\foo"), new File("C:\\bar"), true));
+ }
+
+ @Test
+ public void isLeadingPathAndTrailingSlashesOnDos() throws IOException {
+ Assume.assumeTrue("Test only runs on DOS", PATH_SEP.equals(";"));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\bar"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\bar\\"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo"), true));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\"), true));
+
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\bar"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\bar\\"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo\\"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo\\"), new File("c:\\foo"), false));
+ Assert.assertTrue(FileUtil.isLeadingPath(new File("c:\\foo"), new File("c:\\foo\\"), false));
+ }
}
diff --git a/test/java/org/apache/ivy/core/pack/ZipPackingTest.java b/test/java/org/apache/ivy/core/pack/ZipPackingTest.java
new file mode 100644
index 0000000..5435dff
--- /dev/null
+++ b/test/java/org/apache/ivy/core/pack/ZipPackingTest.java
@@ -0,0 +1,72 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+package org.apache.ivy.core.pack;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+import org.apache.ivy.TestHelper;
+import org.apache.ivy.util.DefaultMessageLogger;
+import org.apache.ivy.util.FileUtil;
+import org.apache.ivy.util.Message;
+import org.apache.tools.ant.Project;
+import org.apache.tools.ant.taskdefs.Mkdir;
+import org.apache.tools.ant.taskdefs.Delete;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ZipPackingTest {
+
+ private static final Project PROJECT = TestHelper.newProject();
+ private static final File TEST_DIR = PROJECT.resolveFile("build/test/pack");
+
+ @Before
+ public void setUp() {
+ Mkdir mkdir = new Mkdir();
+ mkdir.setProject(PROJECT);
+ mkdir.setDir(TEST_DIR);
+ mkdir.execute();
+ Message.setDefaultLogger(new DefaultMessageLogger(Message.MSG_INFO));
+ }
+
+ @After
+ public void tearDown() {
+ Delete del = new Delete();
+ del.setProject(PROJECT);
+ del.setDir(TEST_DIR);
+ del.execute();
+ }
+
+ @Test
+ public void zipPackingExtractsArchive() throws IOException {
+ try (InputStream zip = new FileInputStream(PROJECT.resolveFile("test/zip/test.zip"))) {
+ new ZipPacking().unpack(zip, TEST_DIR);
+ }
+ assertTrue("Expecting file a", FileUtil.resolveFile(TEST_DIR, "a").isFile());
+ assertTrue("Expecting directory b", FileUtil.resolveFile(TEST_DIR, "b").isDirectory());
+ assertTrue("Expecting file b/c", FileUtil.resolveFile(TEST_DIR, "b/c").isFile());
+ assertTrue("Expecting directory d", FileUtil.resolveFile(TEST_DIR, "d").isDirectory());
+ assertFalse("Not expecting file e", PROJECT.resolveFile("build/test/e").exists());
+ }
+}
diff --git a/test/zip/test.zip b/test/zip/test.zip
new file mode 100644
index 0000000..b1b653a
--- /dev/null
+++ b/test/zip/test.zip
Binary files differ
