Skip to content

Commit 6d780b3

Browse files
Copilotslachiewicz
andauthored
Fix Zip Slip vulnerability in archive extraction (#296)
--------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: slachiewicz <[email protected]>
1 parent 8218805 commit 6d780b3

File tree

2 files changed

+161
-2
lines changed

2 files changed

+161
-2
lines changed

src/main/java/org/codehaus/plexus/util/Expand.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,19 @@ protected void extractFile(
111111
throws Exception {
112112
File f = FileUtils.resolveFile(dir, entryName);
113113

114-
if (!f.getAbsolutePath().startsWith(dir.getAbsolutePath())) {
115-
throw new IOException("Entry '" + entryName + "' outside the target directory.");
114+
try {
115+
String canonicalDirPath = dir.getCanonicalPath();
116+
String canonicalFilePath = f.getCanonicalPath();
117+
118+
// Ensure the file is within the target directory
119+
// We need to check that the canonical file path starts with the canonical directory path
120+
// followed by a file separator to prevent path traversal attacks
121+
if (!canonicalFilePath.startsWith(canonicalDirPath + File.separator)
122+
&& !canonicalFilePath.equals(canonicalDirPath)) {
123+
throw new IOException("Entry '" + entryName + "' outside the target directory.");
124+
}
125+
} catch (IOException e) {
126+
throw new IOException("Failed to verify entry path for '" + entryName + "'", e);
116127
}
117128

118129
try {
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
package org.codehaus.plexus.util;
2+
3+
/*
4+
* Copyright The Codehaus Foundation.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
import java.io.File;
20+
import java.nio.file.Files;
21+
import java.util.zip.ZipEntry;
22+
import java.util.zip.ZipOutputStream;
23+
24+
import org.junit.jupiter.api.Test;
25+
26+
import static org.junit.jupiter.api.Assertions.assertFalse;
27+
import static org.junit.jupiter.api.Assertions.assertThrows;
28+
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
30+
/**
31+
* Test for {@link Expand}.
32+
*/
33+
class ExpandTest extends FileBasedTestCase {
34+
35+
@Test
36+
void testZipSlipVulnerabilityWithParentDirectory() throws Exception {
37+
File tempDir = getTestDirectory();
38+
File zipFile = new File(tempDir, "malicious.zip");
39+
File targetDir = new File(tempDir, "extract");
40+
targetDir.mkdirs();
41+
42+
// Create a malicious zip with path traversal
43+
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) {
44+
ZipEntry entry = new ZipEntry("../../evil.txt");
45+
zos.putNextEntry(entry);
46+
zos.write("malicious content".getBytes());
47+
zos.closeEntry();
48+
}
49+
50+
Expand expand = new Expand();
51+
expand.setSrc(zipFile);
52+
expand.setDest(targetDir);
53+
54+
// This should throw an exception, not extract the file
55+
assertThrows(Exception.class, () -> expand.execute());
56+
57+
// Verify the file was not created outside the target directory
58+
File evilFile = new File(tempDir, "evil.txt");
59+
assertFalse(evilFile.exists(), "File should not be extracted outside target directory");
60+
}
61+
62+
@Test
63+
void testZipSlipVulnerabilityWithAbsolutePath() throws Exception {
64+
File tempDir = getTestDirectory();
65+
File zipFile = new File(tempDir, "malicious-absolute.zip");
66+
File targetDir = new File(tempDir, "extract-abs");
67+
targetDir.mkdirs();
68+
69+
// Create a malicious zip with absolute path
70+
File evilTarget = new File("/tmp/evil-absolute.txt");
71+
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) {
72+
ZipEntry entry = new ZipEntry(evilTarget.getAbsolutePath());
73+
zos.putNextEntry(entry);
74+
zos.write("malicious content".getBytes());
75+
zos.closeEntry();
76+
}
77+
78+
Expand expand = new Expand();
79+
expand.setSrc(zipFile);
80+
expand.setDest(targetDir);
81+
82+
// This should throw an exception, not extract the file
83+
assertThrows(Exception.class, () -> expand.execute());
84+
85+
// Verify the file was not created at the absolute path
86+
assertFalse(evilTarget.exists(), "File should not be extracted to absolute path");
87+
}
88+
89+
@Test
90+
void testZipSlipVulnerabilityWithSimilarDirectoryName() throws Exception {
91+
File tempDir = getTestDirectory();
92+
File zipFile = new File(tempDir, "malicious-similar.zip");
93+
File targetDir = new File(tempDir, "extract");
94+
targetDir.mkdirs();
95+
96+
// Create a directory with a similar name to test prefix matching vulnerability
97+
File similarDir = new File(tempDir, "extract-evil");
98+
similarDir.mkdirs();
99+
100+
// Create a malicious zip that tries to exploit prefix matching
101+
// If targetDir is /tmp/extract, this tries to write to /tmp/extract-evil/file.txt
102+
String maliciousPath = "../extract-evil/evil.txt";
103+
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) {
104+
ZipEntry entry = new ZipEntry(maliciousPath);
105+
zos.putNextEntry(entry);
106+
zos.write("malicious content".getBytes());
107+
zos.closeEntry();
108+
}
109+
110+
Expand expand = new Expand();
111+
expand.setSrc(zipFile);
112+
expand.setDest(targetDir);
113+
114+
// This should throw an exception, not extract the file
115+
assertThrows(Exception.class, () -> expand.execute());
116+
117+
// Verify the file was not created in the similar directory
118+
File evilFile = new File(similarDir, "evil.txt");
119+
assertFalse(evilFile.exists(), "File should not be extracted to directory with similar name");
120+
}
121+
122+
@Test
123+
void testNormalZipExtraction() throws Exception {
124+
File tempDir = getTestDirectory();
125+
File zipFile = new File(tempDir, "normal.zip");
126+
File targetDir = new File(tempDir, "extract-normal");
127+
targetDir.mkdirs();
128+
129+
// Create a normal zip
130+
try (ZipOutputStream zos = new ZipOutputStream(Files.newOutputStream(zipFile.toPath()))) {
131+
ZipEntry entry = new ZipEntry("subdir/normal.txt");
132+
zos.putNextEntry(entry);
133+
zos.write("normal content".getBytes());
134+
zos.closeEntry();
135+
}
136+
137+
Expand expand = new Expand();
138+
expand.setSrc(zipFile);
139+
expand.setDest(targetDir);
140+
141+
// This should succeed
142+
expand.execute();
143+
144+
// Verify the file was created in the correct location
145+
File normalFile = new File(targetDir, "subdir/normal.txt");
146+
assertTrue(normalFile.exists(), "File should be extracted to correct location");
147+
}
148+
}

0 commit comments

Comments
 (0)