Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ZipSlip not fixed #55

Closed
AFulgens opened this issue Aug 28, 2019 · 5 comments
Closed

ZipSlip not fixed #55

AFulgens opened this issue Aug 28, 2019 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@AFulgens
Copy link

According to tests and manual debugging, the library is still vulnerable against ZipSlip.

The fix seems to be a change in AbstractExtractFileTask#extractFile(...), namely:

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

should be replaced with

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

Test case (JUnit5 + AspectJ) for verification:

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

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

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

	try {
		zip.extractAll(new File(tempDir, "unzipped").getAbsolutePath());
		fail("zip4j is vulnerable for slip zip");
	}
	catch (ZipException e) {
		assertThat(e).hasMessageStartingWith("illegal file name that breaks out of the target directory: ");
	}
} 
@srikanth-lingala srikanth-lingala self-assigned this Aug 29, 2019
@srikanth-lingala srikanth-lingala added the bug Something isn't working label Aug 29, 2019
@srikanth-lingala
Copy link
Owner

Thanks for pointing it out. I will test and include a fix for this and make a release in the next couple of days

srikanth-lingala added a commit that referenced this issue Sep 5, 2019
@srikanth-lingala
Copy link
Owner

Fixed in v2.1.3 released today

@DamonChen117
Copy link

It cause issue on Android,

completePath = /data/user/0/com.damon.englishbook/cache/temp/unzip/1
outPath = /data/user/0/com.damon.englishbook/cache/temp/unzip/

File(completePath).canonicalPath = /data/data/com.damon.englishbook/cache/temp/unzip/1
File(outPath).path = /data/user/0/com.damon.englishbook/cache/temp/unzip

it then throw exception : "illegal file name that break...."

@srikanth-lingala
Copy link
Owner

@DamonChen117 Did you try to use the latest version v2.1.4? There was another fix done for this in 2.1.4

@DamonChen117
Copy link

@srikanth-lingala Just tried 2.1.4, it works perfect, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants