Mitigate Zip Slip / Path Traversal (CWE-22) Vulnerability in Archive Extraction

bundlerepository/pom.xml

@@ -107,8 +107,8 @@
         <groupId>org.apache.maven.plugins</groupId>
         <artifactId>maven-compiler-plugin</artifactId>
         <configuration>
-          <target>1.5</target>
-          <source>1.5</source>
+          <target>1.8</target>
+          <source>1.8</source>
         </configuration>
       </plugin>
 

bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/FileUtil.java

@@ -101,6 +101,8 @@ public static void unjar(JarInputStream jis, File dir)
         // Reusable buffer.
         byte[] buffer = new byte[4096];
 
+        String canonicalBase = dir.getCanonicalPath();
+
         // Loop through JAR entries.
         for (JarEntry je = jis.getNextJarEntry();
              je != null;
@@ -112,6 +114,11 @@ public static void unjar(JarInputStream jis, File dir)
             }
 
             File target = new File(dir, je.getName());
+            String canonicalTarget = target.getCanonicalPath();
+            if (!canonicalTarget.startsWith(canonicalBase + File.separator) && !canonicalTarget.equals(canonicalBase))
+            {
+                throw new IOException("JAR entry resolves outside the target directory: " + je.getName());
+            }
 
             // Check to see if the JAR entry is a directory.
             if (je.isDirectory())

bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/ObrGogoCommand.java

@@ -754,6 +754,8 @@ public static void unjar(JarInputStream jis, File dir)
         // Reusable buffer.
         byte[] buffer = new byte[4096];
 
+        String canonicalBase = dir.getCanonicalPath();
+
         // Loop through JAR entries.
         for (JarEntry je = jis.getNextJarEntry();
              je != null;
@@ -765,6 +767,11 @@ public static void unjar(JarInputStream jis, File dir)
             }
 
             File target = new File(dir, je.getName());
+            String canonicalTarget = target.getCanonicalPath();
+            if (!canonicalTarget.startsWith(canonicalBase + File.separator) && !canonicalTarget.equals(canonicalBase))
+            {
+                throw new IOException("JAR entry resolves outside the target directory: " + je.getName());
+            }
 
             // Check to see if the JAR entry is a directory.
             if (je.isDirectory())

bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/FileUtilTest.java

@@ -0,0 +1,82 @@
+/*
+ * 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
+ *
+ *   http://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.felix.bundlerepository.impl;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.IOException;
+import java.util.jar.JarEntry;
+import java.util.jar.JarInputStream;
+import java.util.jar.JarOutputStream;
+
+import junit.framework.TestCase;
+
+public class FileUtilTest extends TestCase
+{
+    public void testUnjarZipSlip() throws Exception
+    {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        JarOutputStream jos = new JarOutputStream(baos);
+        try
+        {
+            JarEntry entry = new JarEntry("../../evil.txt");
+            jos.putNextEntry(entry);
+            jos.write("malicious content".getBytes());
+            jos.closeEntry();
+        }
+        finally
+        {
+            jos.close();
+        }
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(baos.toByteArray());
+        JarInputStream jis = new JarInputStream(bais);
+
+        File targetDir = new File("target/extraction-test-fileutil");
+        targetDir.mkdirs();
+
+        try
+        {
+            FileUtil.unjar(jis, targetDir);
+            fail("Expected IOException due to Zip Slip path traversal");
+        }
+        catch (IOException e)
+        {
+            assertTrue(e.getMessage().contains("resolves outside the target directory"));
+        }
+        finally
+        {
+            deleteDirectory(targetDir);
+        }
+    }
+
+    private void deleteDirectory(File dir)
+    {
+        File[] files = dir.listFiles();
+        if (files != null)
+        {
+            for (File f : files)
+            {
+                deleteDirectory(f);
+            }
+        }
+        dir.delete();
+    }
+}

gogo/command/src/main/java/org/apache/felix/gogo/command/Util.java

@@ -155,6 +155,8 @@ public static void unjar(JarInputStream jis, File dir)
         // Reusable buffer.
         byte[] buffer = new byte[4096];
 
+        String canonicalBase = dir.getCanonicalPath();
+
         // Loop through JAR entries.
         for (JarEntry je = jis.getNextJarEntry();
              je != null;
@@ -166,6 +168,11 @@ public static void unjar(JarInputStream jis, File dir)
             }
 
             File target = new File(dir, je.getName());
+            String canonicalTarget = target.getCanonicalPath();
+            if (!canonicalTarget.startsWith(canonicalBase + File.separator) && !canonicalTarget.equals(canonicalBase))
+            {
+                throw new IOException("JAR entry resolves outside the target directory: " + je.getName());
+            }
 
             // Check to see if the JAR entry is a directory.
             if (je.isDirectory())

gogo/command/src/test/java/org/apache/felix/gogo/command/UtilTest.java

@@ -46,4 +46,40 @@ public void relativeUri() throws Exception {
         u = Util.resolveUri(session, "three");
         assertEquals(new File("./three").getCanonicalFile().toURI().toString(), u);
     }
+
+    @Test
+    public void testUnjarZipSlip() throws Exception {
+        java.io.ByteArrayOutputStream baos = new java.io.ByteArrayOutputStream();
+        try (java.util.jar.JarOutputStream jos = new java.util.jar.JarOutputStream(baos)) {
+            java.util.jar.JarEntry entry = new java.util.jar.JarEntry("../../evil.txt");
+            jos.putNextEntry(entry);
+            jos.write("malicious content".getBytes());
+            jos.closeEntry();
+        }
+
+        java.io.ByteArrayInputStream bais = new java.io.ByteArrayInputStream(baos.toByteArray());
+        java.util.jar.JarInputStream jis = new java.util.jar.JarInputStream(bais);
+
+        File targetDir = new File("target/extraction-test-util");
+        targetDir.mkdirs();
+
+        try {
+            Util.unjar(jis, targetDir);
+            org.junit.Assert.fail("Expected IOException due to Zip Slip path traversal");
+        } catch (java.io.IOException e) {
+            org.junit.Assert.assertTrue(e.getMessage().contains("resolves outside the target directory"));
+        } finally {
+            deleteDirectory(targetDir);
+        }
+    }
+
+    private void deleteDirectory(File dir) {
+        File[] files = dir.listFiles();
+        if (files != null) {
+            for (File f : files) {
+                deleteDirectory(f);
+            }
+        }
+        dir.delete();
+    }
 }