CVE-2022-37865 ZipPacking allows overwriting arbitrary files

src/java/org/apache/ivy/core/pack/ZipPacking.java

@@ -52,8 +52,15 @@ public void unpack(InputStream packed, File dest) throws IOException {
         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();

src/java/org/apache/ivy/util/FileUtil.java

@@ -610,6 +610,68 @@ private static DissectedPath dissect(final String path) {
         return new DissectedPath(File.separator, pathToDissect);
     }
 
+    /**
+     * 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
      *

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 @@ public void testCopyOfSameFile() throws Exception {
         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));
+    }
 }

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());
+    }
+}