|
| 1 | +From: Copilot <198982749+Copilot@users.noreply.github.com> |
| 2 | +Date: Sun, 9 Nov 2025 12:32:38 +0100 |
| 3 | +Subject: [PATCH] Fix Zip Slip vulnerability in archive extraction (#296) |
| 4 | + |
| 5 | +--------- |
| 6 | +Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> |
| 7 | +Co-authored-by: slachiewicz <6705942+slachiewicz@users.noreply.github.com> |
| 8 | + |
| 9 | +Upstream Patch Reference: https://github.com/codehaus-plexus/plexus-utils/commit/6d780b3378829318ba5c2d29547e0012d5b29642.patch |
| 10 | +--- |
| 11 | + .../java/org/codehaus/plexus/util/Expand.java | 18 ++- |
| 12 | + .../org/codehaus/plexus/util/ExpandTest.java | 148 ++++++++++++++++++ |
| 13 | + 2 files changed, 162 insertions(+), 4 deletions(-) |
| 14 | + create mode 100644 src/test/java/org/codehaus/plexus/util/ExpandTest.java |
| 15 | + |
| 16 | +diff --git a/src/main/java/org/codehaus/plexus/util/Expand.java b/src/main/java/org/codehaus/plexus/util/Expand.java |
| 17 | +index 3d3e98a..512ee83 100644 |
| 18 | +--- a/src/main/java/org/codehaus/plexus/util/Expand.java |
| 19 | ++++ b/src/main/java/org/codehaus/plexus/util/Expand.java |
| 20 | +@@ -136,10 +136,20 @@ protected void extractFile( File srcF, File dir, InputStream compressedInputStre |
| 21 | + { |
| 22 | + File f = FileUtils.resolveFile( dir, entryName ); |
| 23 | + |
| 24 | +- if ( !f.getAbsolutePath().startsWith( dir.getAbsolutePath() ) ) |
| 25 | +- { |
| 26 | +- throw new IOException( "Entry '" + entryName + "' outside the target directory." ); |
| 27 | +- } |
| 28 | ++ try { |
| 29 | ++ String canonicalDirPath = dir.getCanonicalPath(); |
| 30 | ++ String canonicalFilePath = f.getCanonicalPath(); |
| 31 | ++ |
| 32 | ++ // Ensure the file is within the target directory |
| 33 | ++ // We need to check that the canonical file path starts with the canonical directory path |
| 34 | ++ // followed by a file separator to prevent path traversal attacks |
| 35 | ++ if (!canonicalFilePath.startsWith(canonicalDirPath + File.separator) |
| 36 | ++ && !canonicalFilePath.equals(canonicalDirPath)) { |
| 37 | ++ throw new IOException("Entry '" + entryName + "' outside the target directory."); |
| 38 | ++ } |
| 39 | ++ } catch (IOException e) { |
| 40 | ++ throw new IOException("Failed to verify entry path for '" + entryName + "'", e); |
| 41 | ++ } |
| 42 | + |
| 43 | + try |
| 44 | + { |
| 45 | +diff --git a/src/test/java/org/codehaus/plexus/util/ExpandTest.java b/src/test/java/org/codehaus/plexus/util/ExpandTest.java |
| 46 | +new file mode 100644 |
| 47 | +index 0000000..46e3d0e |
| 48 | +--- /dev/null |
| 49 | ++++ b/src/test/java/org/codehaus/plexus/util/ExpandTest.java |
| 50 | +@@ -0,0 +1,148 @@ |
| 51 | ++package org.codehaus.plexus.util; |
| 52 | ++ |
| 53 | ++/* |
| 54 | ++ * Copyright The Codehaus Foundation. |
| 55 | ++ * |
| 56 | ++ * Licensed under the Apache License, Version 2.0 (the "License"); |
| 57 | ++ * you may not use this file except in compliance with the License. |
| 58 | ++ * You may obtain a copy of the License at |
| 59 | ++ * |
| 60 | ++ * http://www.apache.org/licenses/LICENSE-2.0 |
| 61 | ++ * |
| 62 | ++ * Unless required by applicable law or agreed to in writing, software |
| 63 | ++ * distributed under the License is distributed on an "AS IS" BASIS, |
| 64 | ++ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. |
| 65 | ++ * See the License for the specific language governing permissions and |
| 66 | ++ * limitations under the License. |
| 67 | ++ */ |
| 68 | ++ |
| 69 | ++import java.io.File; |
| 70 | ++import java.nio.file.Files; |
| 71 | ++import java.util.zip.ZipEntry; |
| 72 | ++import java.util.zip.ZipOutputStream; |
| 73 | ++ |
| 74 | ++import org.junit.jupiter.api.Test; |
| 75 | ++ |
| 76 | ++import static org.junit.jupiter.api.Assertions.assertFalse; |
| 77 | ++import static org.junit.jupiter.api.Assertions.assertThrows; |
| 78 | ++import static org.junit.jupiter.api.Assertions.assertTrue; |
| 79 | ++ |
| 80 | ++/** |
| 81 | ++ * Test for {@link Expand}. |
| 82 | ++ */ |
| 83 | ++class ExpandTest extends FileBasedTestCase { |
| 84 | ++ |
| 85 | ++ @Test |
| 86 | ++ void testZipSlipVulnerabilityWithParentDirectory() throws Exception { |
| 87 | ++ File tempDir = getTestDirectory(); |
| 88 | ++ File zipFile = new File(tempDir, "malicious.zip"); |
| 89 | ++ File targetDir = new File(tempDir, "extract"); |
| 90 | ++ targetDir.mkdirs(); |
| 91 | ++ |
| 92 | ++ // Create a malicious zip with path traversal |
| 93 | ++ try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) { |
| 94 | ++ ZipEntry entry = new ZipEntry("../../evil.txt"); |
| 95 | ++ zos.putNextEntry(entry); |
| 96 | ++ zos.write("malicious content".getBytes()); |
| 97 | ++ zos.closeEntry(); |
| 98 | ++ } |
| 99 | ++ |
| 100 | ++ Expand expand = new Expand(); |
| 101 | ++ expand.setSrc(zipFile); |
| 102 | ++ expand.setDest(targetDir); |
| 103 | ++ |
| 104 | ++ // This should throw an exception, not extract the file |
| 105 | ++ assertThrows(Exception.class, () -> expand.execute()); |
| 106 | ++ |
| 107 | ++ // Verify the file was not created outside the target directory |
| 108 | ++ File evilFile = new File(tempDir, "evil.txt"); |
| 109 | ++ assertFalse(evilFile.exists(), "File should not be extracted outside target directory"); |
| 110 | ++ } |
| 111 | ++ |
| 112 | ++ @Test |
| 113 | ++ void testZipSlipVulnerabilityWithAbsolutePath() throws Exception { |
| 114 | ++ File tempDir = getTestDirectory(); |
| 115 | ++ File zipFile = new File(tempDir, "malicious-absolute.zip"); |
| 116 | ++ File targetDir = new File(tempDir, "extract-abs"); |
| 117 | ++ targetDir.mkdirs(); |
| 118 | ++ |
| 119 | ++ // Create a malicious zip with absolute path |
| 120 | ++ File evilTarget = new File("/tmp/evil-absolute.txt"); |
| 121 | ++ try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) { |
| 122 | ++ ZipEntry entry = new ZipEntry(evilTarget.getAbsolutePath()); |
| 123 | ++ zos.putNextEntry(entry); |
| 124 | ++ zos.write("malicious content".getBytes()); |
| 125 | ++ zos.closeEntry(); |
| 126 | ++ } |
| 127 | ++ |
| 128 | ++ Expand expand = new Expand(); |
| 129 | ++ expand.setSrc(zipFile); |
| 130 | ++ expand.setDest(targetDir); |
| 131 | ++ |
| 132 | ++ // This should throw an exception, not extract the file |
| 133 | ++ assertThrows(Exception.class, () -> expand.execute()); |
| 134 | ++ |
| 135 | ++ // Verify the file was not created at the absolute path |
| 136 | ++ assertFalse(evilTarget.exists(), "File should not be extracted to absolute path"); |
| 137 | ++ } |
| 138 | ++ |
| 139 | ++ @Test |
| 140 | ++ void testZipSlipVulnerabilityWithSimilarDirectoryName() throws Exception { |
| 141 | ++ File tempDir = getTestDirectory(); |
| 142 | ++ File zipFile = new File(tempDir, "malicious-similar.zip"); |
| 143 | ++ File targetDir = new File(tempDir, "extract"); |
| 144 | ++ targetDir.mkdirs(); |
| 145 | ++ |
| 146 | ++ // Create a directory with a similar name to test prefix matching vulnerability |
| 147 | ++ File similarDir = new File(tempDir, "extract-evil"); |
| 148 | ++ similarDir.mkdirs(); |
| 149 | ++ |
| 150 | ++ // Create a malicious zip that tries to exploit prefix matching |
| 151 | ++ // If targetDir is /tmp/extract, this tries to write to /tmp/extract-evil/file.txt |
| 152 | ++ String maliciousPath = "../extract-evil/evil.txt"; |
| 153 | ++ try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) { |
| 154 | ++ ZipEntry entry = new ZipEntry(maliciousPath); |
| 155 | ++ zos.putNextEntry(entry); |
| 156 | ++ zos.write("malicious content".getBytes()); |
| 157 | ++ zos.closeEntry(); |
| 158 | ++ } |
| 159 | ++ |
| 160 | ++ Expand expand = new Expand(); |
| 161 | ++ expand.setSrc(zipFile); |
| 162 | ++ expand.setDest(targetDir); |
| 163 | ++ |
| 164 | ++ // This should throw an exception, not extract the file |
| 165 | ++ assertThrows(Exception.class, () -> expand.execute()); |
| 166 | ++ |
| 167 | ++ // Verify the file was not created in the similar directory |
| 168 | ++ File evilFile = new File(similarDir, "evil.txt"); |
| 169 | ++ assertFalse(evilFile.exists(), "File should not be extracted to directory with similar name"); |
| 170 | ++ } |
| 171 | ++ |
| 172 | ++ @Test |
| 173 | ++ void testNormalZipExtraction() throws Exception { |
| 174 | ++ File tempDir = getTestDirectory(); |
| 175 | ++ File zipFile = new File(tempDir, "normal.zip"); |
| 176 | ++ File targetDir = new File(tempDir, "extract-normal"); |
| 177 | ++ targetDir.mkdirs(); |
| 178 | ++ |
| 179 | ++ // Create a normal zip |
| 180 | ++ try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) { |
| 181 | ++ ZipEntry entry = new ZipEntry("subdir/normal.txt"); |
| 182 | ++ zos.putNextEntry(entry); |
| 183 | ++ zos.write("normal content".getBytes()); |
| 184 | ++ zos.closeEntry(); |
| 185 | ++ } |
| 186 | ++ |
| 187 | ++ Expand expand = new Expand(); |
| 188 | ++ expand.setSrc(zipFile); |
| 189 | ++ expand.setDest(targetDir); |
| 190 | ++ |
| 191 | ++ // This should succeed |
| 192 | ++ expand.execute(); |
| 193 | ++ |
| 194 | ++ // Verify the file was created in the correct location |
| 195 | ++ File normalFile = new File(targetDir, "subdir/normal.txt"); |
| 196 | ++ assertTrue(normalFile.exists(), "File should be extracted to correct location"); |
| 197 | ++ } |
| 198 | ++} |
| 199 | +-- |
| 200 | +2.45.4 |
| 201 | + |
0 commit comments