Skip to content

Commit

Permalink
Improve malicious directory traversal check
Browse files Browse the repository at this point in the history
  • Loading branch information
trgpa committed Mar 4, 2021
1 parent 8175140 commit 68268f6
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/main/java/org/zeroturnaround/zip/ZipUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -1140,7 +1140,8 @@ private static File checkDestinationFileForTraversal(File outputDir, String name
* that the outputdir + name doesn't leave the outputdir. See
* DirectoryTraversalMaliciousTest for details.
*/
if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(outputDir.getCanonicalPath())) {
String canonicalOutputPath = outputDir.getCanonicalPath() + File.separator;
if (name.indexOf("..") != -1 && !destFile.getCanonicalPath().startsWith(canonicalOutputPath)) {
throw new MaliciousZipException(outputDir, name);
}
return destFile;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,18 @@ public class DirectoryTraversalMaliciousTest extends TestCase {
*/
private static final File badFileBackslashes = new File("src/test/resources/zip-malicious-traversal-backslashes.zip");

/*
* This is the contents of the file. There is one evil file that tries to get out of the
* target.
*
* $ unzip -t zip-malicious-traversal-same-prefix.zip
* Archive: zip-malicious-traversal-same-prefix.zip
* testing: good.txt OK
* testing: ../foobar/evil.txt OK
* No errors detected in compressed data of zip-malicious-traversal-same-prefix.zip.
*/
private static final File badFileSamePrefix = new File("src/test/resources/zip-malicious-traversal-same-prefix.zip");

public void testUnpackDoesntLeaveTarget() throws Exception {
File file = File.createTempFile("temp", null);
File tmpDir = file.getParentFile();
Expand Down Expand Up @@ -94,4 +106,19 @@ public void testBackslashUnpackerDoesntLeaveTarget() throws Exception {
assertTrue(true);
}
}

public void testSamePrefixDoesntLeaveTarget() throws Exception {
File file = File.createTempFile("temp", null);
File tmpDir = file.getParentFile();
// foobar/evil.txt has the same prefix with the target foo
File target = new File(tmpDir, "foo");
try {
ZipUtil.unpack(badFileSamePrefix, target);
// evil.txt should not be unpacked to tmpDir/foobar/evil.txt
fail();
}
catch (MaliciousZipException e) {
assertTrue(true);
}
}
}
Binary file not shown.

0 comments on commit 68268f6

Please sign in to comment.