diff --git a/bundlerepository/pom.xml b/bundlerepository/pom.xml index be6c3ebf8a..ba938c8462 100644 --- a/bundlerepository/pom.xml +++ b/bundlerepository/pom.xml @@ -107,8 +107,8 @@ org.apache.maven.plugins maven-compiler-plugin - 1.5 - 1.5 + 1.8 + 1.8 diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/FileUtil.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/FileUtil.java index 9a27e3ff49..c5876a561b 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/FileUtil.java +++ b/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()) diff --git a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/ObrGogoCommand.java b/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/ObrGogoCommand.java index 4d3f7a6ebe..81c5e4864b 100644 --- a/bundlerepository/src/main/java/org/apache/felix/bundlerepository/impl/ObrGogoCommand.java +++ b/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()) diff --git a/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/FileUtilTest.java b/bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/FileUtilTest.java new file mode 100644 index 0000000000..128bd05fd7 --- /dev/null +++ b/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(); + } +} diff --git a/gogo/command/src/main/java/org/apache/felix/gogo/command/Util.java b/gogo/command/src/main/java/org/apache/felix/gogo/command/Util.java index 21a84272a1..fc0ac73a47 100644 --- a/gogo/command/src/main/java/org/apache/felix/gogo/command/Util.java +++ b/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()) diff --git a/gogo/command/src/test/java/org/apache/felix/gogo/command/UtilTest.java b/gogo/command/src/test/java/org/apache/felix/gogo/command/UtilTest.java index 89008cf744..e779771fc8 100644 --- a/gogo/command/src/test/java/org/apache/felix/gogo/command/UtilTest.java +++ b/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(); + } }