From 989fac860a187b8b212ed8eb107dbedb30f5b895 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Tue, 2 Jun 2026 12:20:07 +0530 Subject: [PATCH 1/3] Fix: Mitigate Zip Slip / Path Traversal (CWE-22) vulnerabilities during archive extraction --- bundlerepository/pom.xml | 4 ++-- .../org/apache/felix/bundlerepository/impl/FileUtil.java | 7 +++++++ .../apache/felix/bundlerepository/impl/ObrGogoCommand.java | 7 +++++++ .../src/main/java/org/apache/felix/gogo/command/Util.java | 7 +++++++ 4 files changed, 23 insertions(+), 2 deletions(-) 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/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()) From 3d0f04977565fdf5bd004b25b15281469c48c583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Tue, 2 Jun 2026 12:27:29 +0530 Subject: [PATCH 2/3] Test: Add regression tests for Zip Slip path traversal mitigation --- .../bundlerepository/impl/FileUtilTest.java | 82 +++++++++++++++++++ .../apache/felix/gogo/command/UtilTest.java | 36 ++++++++ 2 files changed, 118 insertions(+) create mode 100644 bundlerepository/src/test/java/org/apache/felix/bundlerepository/impl/FileUtilTest.java 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/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(); + } } From 4efe39c0074d34b960c335f1fed9731ec0055ba5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Csahvx655-wq=E2=80=9D?= <“sahvx655@gmail.com”> Date: Mon, 15 Jun 2026 11:26:00 +0530 Subject: [PATCH 3/3] fix: mitigate path traversal (Zip Slip) vulnerability in cache handlers --- .../cache/ConnectContentContent.java | 13 ++ .../framework/cache/DirectoryContent.java | 18 ++- .../felix/framework/cache/JarContent.java | 18 ++- .../framework/cache/BundleCacheTest.java | 145 ++++++++++++++---- 4 files changed, 151 insertions(+), 43 deletions(-) diff --git a/framework/src/main/java/org/apache/felix/framework/cache/ConnectContentContent.java b/framework/src/main/java/org/apache/felix/framework/cache/ConnectContentContent.java index c64ee15089..c1433c7f28 100644 --- a/framework/src/main/java/org/apache/felix/framework/cache/ConnectContentContent.java +++ b/framework/src/main/java/org/apache/felix/framework/cache/ConnectContentContent.java @@ -146,6 +146,19 @@ public Content getEntryAsContent(String name) { return this; } + + // Remove any leading slash. + String entryName = (name.startsWith("/")) ? name.substring(1) : name; + + String normalizedEntryName = entryName.replace('\\', '/'); + if (normalizedEntryName.trim().startsWith("../") || + normalizedEntryName.contains("/../") || + normalizedEntryName.trim().endsWith("/..") || + normalizedEntryName.trim().equals("..")) + { + return null; + } + String dir = name.endsWith("/") ? name : name + "/"; if (hasEntry(dir)) diff --git a/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java b/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java index 11dfe0c8a3..f6bb9a2f75 100644 --- a/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java +++ b/framework/src/main/java/org/apache/felix/framework/cache/DirectoryContent.java @@ -252,10 +252,11 @@ public Content getEntryAsContent(String entryName) // entries are relative to the root of the bundle. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; - if (entryName.trim().startsWith(".." + File.separatorChar) || - entryName.contains(File.separator + ".." + File.separatorChar) || - entryName.trim().endsWith(File.separator + "..") || - entryName.trim().equals("..")) + String normalizedEntryName = entryName.replace('\\', '/'); + if (normalizedEntryName.trim().startsWith("../") || + normalizedEntryName.contains("/../") || + normalizedEntryName.trim().endsWith("/..") || + normalizedEntryName.trim().equals("..")) { return null; } @@ -305,10 +306,11 @@ public String getEntryAsNativeLibrary(String entryName) // entries are relative to the root of the bundle. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; - if (entryName.trim().startsWith(".." + File.separatorChar) || - entryName.contains(File.separator + ".." + File.separatorChar) || - entryName.trim().endsWith(File.separator + "..") || - entryName.trim().equals("..")) + String normalizedEntryName = entryName.replace('\\', '/'); + if (normalizedEntryName.trim().startsWith("../") || + normalizedEntryName.contains("/../") || + normalizedEntryName.trim().endsWith("/..") || + normalizedEntryName.trim().equals("..")) { return null; } diff --git a/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java b/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java index 161da65bf9..54766b0014 100644 --- a/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java +++ b/framework/src/main/java/org/apache/felix/framework/cache/JarContent.java @@ -241,10 +241,11 @@ public Content getEntryAsContent(String entryName) // Remove any leading slash. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; - if (entryName.trim().startsWith(".." + File.separatorChar) || - entryName.contains(File.separator + ".." + File.separatorChar) || - entryName.trim().endsWith(File.separator + "..") || - entryName.trim().equals("..")) + String normalizedEntryName = entryName.replace('\\', '/'); + if (normalizedEntryName.trim().startsWith("../") || + normalizedEntryName.contains("/../") || + normalizedEntryName.trim().endsWith("/..") || + normalizedEntryName.trim().equals("..")) { return null; } @@ -320,10 +321,11 @@ public String getEntryAsNativeLibrary(String entryName) // Remove any leading slash. entryName = (entryName.startsWith("/")) ? entryName.substring(1) : entryName; - if (entryName.trim().startsWith(".." + File.separatorChar) || - entryName.contains(File.separator + ".." + File.separatorChar) || - entryName.trim().endsWith(File.separator + "..") || - entryName.trim().equals("..")) + String normalizedEntryName = entryName.replace('\\', '/'); + if (normalizedEntryName.trim().startsWith("../") || + normalizedEntryName.contains("/../") || + normalizedEntryName.trim().endsWith("/..") || + normalizedEntryName.trim().equals("..")) { return null; } diff --git a/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java b/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java index 155455731f..bb55685cbc 100644 --- a/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java +++ b/framework/src/test/java/org/apache/felix/framework/cache/BundleCacheTest.java @@ -33,9 +33,11 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; +import java.util.ArrayList; import java.util.Arrays; import java.util.Enumeration; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; import java.util.TreeSet; @@ -45,6 +47,7 @@ class BundleCacheTest { + private final List archives = new ArrayList<>(); private File tempDir; private File cacheDir; private File filesDir; @@ -52,6 +55,19 @@ class BundleCacheTest private File archiveFile; private File jarFile; + private static final String SPECIAL_JAR_ENTRY = File.separatorChar == '\\' + ? "inner/i+~äö §$%nner.jar" + : "inner/i+~äö \\§$%nner.jar"; + + private BundleArchive track(BundleArchive archive) + { + if (archive != null) + { + archives.add(archive); + } + return archive; + } + @BeforeEach void setUp() throws Exception { @@ -85,7 +101,7 @@ protected void doLog(int level, String msg, Throwable throwable) { new File(innerArchiveFile, "empty").mkdirs(); - createJar(archiveFile, new File(archiveFile, "inner/i+?äö \\§$%nner.jar")); + createJar(archiveFile, new File(archiveFile, SPECIAL_JAR_ENTRY)); Manifest manifest = new Manifest(); manifest.getMainAttributes().putValue("foo", "bar"); @@ -113,21 +129,28 @@ void noZipSlip() throws Exception output.putNextEntry(new ZipEntry("../../bar.jar")); - output.write(BundleCache.read(new FileInputStream(jarFile), jarFile.length())); + try (FileInputStream fis = new FileInputStream(jarFile)) + { + output.write(BundleCache.read(fis, jarFile.length())); + } output.closeEntry(); output.close(); - BundleArchive archive = cache.create(1, 1, "slip", new FileInputStream(bundle), null); + BundleArchive archive; + try (FileInputStream fis = new FileInputStream(bundle)) + { + archive = track(cache.create(1, 1, "slip", fis, null)); + } noZipSlip(archive); - archive = cache.create(1, 1, bundle.toURI().toURL().toString(), null, null); + archive = track(cache.create(1, 1, bundle.toURI().toURL().toString(), null, null)); noZipSlip(archive); - archive = cache.create(1, 1, "reference:" + bundle.toURI().toURL().toString(), null, null); + archive = track(cache.create(1, 1, "reference:" + bundle.toURI().toURL().toString(), null, null)); noZipSlip(archive); @@ -139,7 +162,7 @@ void noZipSlip() throws Exception test.createNewFile(); test.deleteOnExit(); - archive = cache.create(1, 1, "reference:" + dir.toURI().toURL().toString(), null, null); + archive = track(cache.create(1, 1, "reference:" + dir.toURI().toURL().toString(), null, null)); noZipSlip(archive); } @@ -189,7 +212,18 @@ void inputStream() throws Exception @Test private BundleArchive bundle(String location, File file) throws Exception { - BundleArchive archive = cache.create(1, 1, location, file != null ? new FileInputStream(file) : null, null); + BundleArchive archive; + if (file != null) + { + try (FileInputStream fis = new FileInputStream(file)) + { + archive = track(cache.create(1, 1, location, fis, null)); + } + } + else + { + archive = track(cache.create(1, 1, location, null, null)); + } assertThat(archive).isNotNull(); @@ -202,7 +236,17 @@ private BundleArchive bundle(String location, File file) throws Exception assertThat(nativeLib).isNotNull(); assertThat(new File(nativeLib)).isFile(); - archive.revise(location, file != null ? new FileInputStream(file) : null); + if (file != null) + { + try (FileInputStream fis = new FileInputStream(file)) + { + archive.revise(location, fis); + } + } + else + { + archive.revise(location, null); + } assertThat(archive.getCurrentRevisionNumber()).isEqualTo(1L); @@ -239,12 +283,13 @@ private void revision(BundleArchive archive) throws Exception assertThat(revision).isNotNull(); assertThat(revision.getManifestHeader()).isNotNull(); assertThat(revision.getManifestHeader()).containsEntry("foo", "bar"); - perRevision(revision.getContent(), new TreeSet<>(Arrays.asList("META-INF/", "META-INF/MANIFEST.MF", "file1", "inner/", "inner/empty/", "inner/file1", "inner/i+?äö \\§$%nner.jar"))); - perRevision(revision.getContent().getEntryAsContent("inner"), new TreeSet<>(Arrays.asList("file1", "empty/", "i+?äö \\§$%nner.jar"))); + String specialEntry = SPECIAL_JAR_ENTRY.replace(File.separatorChar, '/'); + perRevision(revision.getContent(), new TreeSet<>(Arrays.asList("META-INF/", "META-INF/MANIFEST.MF", "file1", "inner/", "inner/empty/", "inner/file1", specialEntry))); + perRevision(revision.getContent().getEntryAsContent("inner"), new TreeSet<>(Arrays.asList("file1", "empty/", specialEntry.substring("inner/".length())))); assertThat(revision.getContent().getEntryAsContent("inner/inner")).isNull(); assertThat(revision.getContent().getEntryAsContent("inner/empty/")).isNotNull(); assertThat(revision.getContent().getEntryAsContent("inner/empty").getEntries()).isNull(); - perRevision(revision.getContent().getEntryAsContent("inner/").getEntryAsContent("i+?äö \\§$%nner.jar"), new TreeSet<>(Arrays.asList("file1", "inner/", "inner/empty/", "inner/file1"))); + perRevision(revision.getContent().getEntryAsContent("inner/").getEntryAsContent(specialEntry.substring("inner/".length())), new TreeSet<>(Arrays.asList("file1", "inner/", "inner/empty/", "inner/file1"))); } private void perRevision(Content content, Set expectedEntries) throws Exception @@ -269,29 +314,35 @@ private void perRevision(Content content, Set expectedEntries) throws Ex assertThat(content.getEntryAsBytes("foo/bar")).isNull(); - InputStream input = content.getEntryAsStream("file1"); - assertThat(input).isNotNull(); - entry = new byte[1014]; - int j = 0; - for (int i = input.read();i != -1; i = input.read()) + try (InputStream input = content.getEntryAsStream("file1")) { - entry[j++] = (byte) i; + assertThat(input).isNotNull(); + entry = new byte[1014]; + int j = 0; + for (int i = input.read();i != -1; i = input.read()) + { + entry[j++] = (byte) i; + } + assertThat(new String(entry, 0, j, "UTF-8")).isEqualTo("file1"); } - assertThat(new String(entry, 0, j, "UTF-8")).isEqualTo("file1"); assertThat(content.getEntryAsStream("foo")).isNull(); assertThat(content.getEntryAsStream("foo/bar")).isNull(); URL url = content.getEntryAsURL("file1"); assertThat(url).isNotNull(); - input = url.openStream(); - assertThat(input).isNotNull(); - entry = new byte[1014]; - j = 0; - for (int i = input.read();i != -1; i = input.read()) + java.net.URLConnection conn = url.openConnection(); + conn.setUseCaches(false); + try (InputStream input = conn.getInputStream()) { - entry[j++] = (byte) i; + assertThat(input).isNotNull(); + entry = new byte[1014]; + int j = 0; + for (int i = input.read();i != -1; i = input.read()) + { + entry[j++] = (byte) i; + } + assertThat(new String(entry, 0, j, "UTF-8")).isEqualTo("file1"); } - assertThat(new String(entry, 0, j, "UTF-8")).isEqualTo("file1"); assertThat(content.getEntryAsURL("foo")).isNull(); assertThat(content.getEntryAsURL("foo/bar")).isNull(); @@ -304,11 +355,45 @@ private void perRevision(Content content, Set expectedEntries) throws Ex @AfterEach void tearDown() throws Exception { + for (BundleArchive archive : archives) + { + try + { + archive.close(); + } + catch (Exception e) + { + // ignore + } + } + archives.clear(); + cache.release(); cache.delete(); + if (cacheDir.exists()) + { + System.out.println("--- CACHE DIR DELETION FAILED. FILES REMAINING: ---"); + printFiles(cacheDir); + } assertThat(cacheDir.exists()).isFalse(); assertThat(BundleCache.deleteDirectoryTree(tempDir)).isTrue(); } + private void printFiles(File dir) + { + File[] files = dir.listFiles(); + if (files != null) + { + for (File f : files) + { + System.out.println(f.getAbsolutePath()); + if (f.isDirectory()) + { + printFiles(f); + } + } + } + } + private void createTestArchive(File archiveFile) throws Exception { createFile(archiveFile, "file1", "file1".getBytes("UTF-8")); @@ -334,7 +419,10 @@ private void createJar(File source, File target) throws Exception JarOutputStream output; if (new File(source, "META-INF/MANIFEST.MF").isFile()) { - output = new JarOutputStream(new FileOutputStream(tmp),new Manifest(new FileInputStream(new File(source, "META-INF/MANIFEST.MF")))); + try (FileInputStream fis = new FileInputStream(new File(source, "META-INF/MANIFEST.MF"))) + { + output = new JarOutputStream(new FileOutputStream(tmp), new Manifest(fis)); + } } else { @@ -376,7 +464,10 @@ private void writeRecursive(File current, String path, JarOutputStream output) t } else if (current.isFile()) { - output.write(BundleCache.read(new FileInputStream(current), current.length())); + try (FileInputStream fis = new FileInputStream(current)) + { + output.write(BundleCache.read(fis, current.length())); + } output.closeEntry(); } }