False-positive result while checking for slip zip if target path is not normalized

[repolevedavaj]

If i use non normalized target paths while unzipping, the check for slip zip ends with a false-positive result.
The issue is, that the path of the extracted file is compared with a non normalized target path in AbstractExtractFileTask#extractFile(...):

if (!new File(completePath).getCanonicalPath().startsWith(new File(outPath).getPath())) {

This line should be changed to

!new File(completePath).getCanonicalPath().startsWith(new File(outPath).getCanonicalPath())

Test case (JUnit5 + AssertJ) for verification:

@Test
public void testUnzipFileZipSlipWithNotNormalizedTarget(@TempDir File tempDir) throws Exception {
	final File goodFile = new File(tempDir, "good.txt");
	assertThat(goodFile.createNewFile()).isTrue();
	FileUtils.write(goodFile, "good", StandardCharsets.UTF_8);

	final ZipParameters zipParameters = new ZipParameters();
	zipParameters.setFileNameInZip("good.txt");

	final ZipFile zip = new ZipFile(new File(tempDir, "test.zip"));
	zip.addFile(goodFile, zipParameters);

	zip.extractAll(new File(tempDir, "../" + tempDir.getName() + "/unzipped").getAbsolutePath());
}

[LeeYoung624]

I tested this case and I think it's right. Only to modify the testcase a little bit:

@Test
  public void testUnzipFileZipSlipWithNotNormalizedTarget() throws IOException {
    ZipFile zipFile = new ZipFile(generatedZipFile, PASSWORD);
    zipFile.addFiles(FILES_TO_ADD);

    try {
      zipFile.extractAll(new File(outputFolder.getPath(),
              ".." + InternalZipConstants.FILE_SEPARATOR + outputFolder.getName()).getAbsolutePath());
    } catch (ZipException e) {
      fail("Should not throw an exception");
    }
  }

Further more, I found other exceptions like this with test case:

@Test
  public void testAddFolderAndFileWithCanonicalPath() throws IOException {
    ZipFile zipFile = new ZipFile(generatedZipFile);
    ZipParameters parameters = new ZipParameters();

    String folderToAddPath = TestUtils.getTestFileFromResources("").getPath() + InternalZipConstants.FILE_SEPARATOR + ".." + InternalZipConstants.FILE_SEPARATOR + TestUtils.getTestFileFromResources("").getName();
    File folderToAdd = new File(folderToAddPath);
    zipFile.addFolder(folderToAdd, parameters);
    zipFile.addFile(TestUtils.getTestFileFromResources("file_PDF_1MB.pdf"), parameters);
    ZipFileVerifier.verifyZipFileByExtractingAllFiles(generatedZipFile, outputFolder, 13);
  }

I'm trying to fix all these.

[srikanth-lingala]

@LeeYoung624 Along with this change, can you please also revert all the getCanonicalPath() changes in this commit and revert it back to getPath() as it was earlier. For example on this line? Thanks.

[srikanth-lingala]

@LeeYoung624 Just to be clear, please only revert the changes in that commit and only from the tests in that commit. Thanks.

[LeeYoung624]

Sure @srikanth-lingala

[cbgxhub]

Hi srikanth-lingala

I ran into the same Problem trying to unzip a zip file that was ziped on Windows but trying to unzip it on Linux, and came to the same conclusion regarding a fix as LeeYoung624:

!new File(completePath).getCanonicalPath().startsWith(new File(outPath).getCanonicalPath())

Regards

[srikanth-lingala]

@cbgxhub A pull request for this issue is already open and work-in-progress. I will make a new release as soon as it is merged.

[srikanth-lingala]

Issue fixed in v2.1.4 released today.