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

Did this fix a security vulnerability? #441

Open
JLLeitschuh opened this issue Jun 23, 2022 · 1 comment
Open

Did this fix a security vulnerability? #441

JLLeitschuh opened this issue Jun 23, 2022 · 1 comment

Comments

@JLLeitschuh
Copy link
Contributor

#400

Reaching out to ask if this fixed a security vulnerability, or was it just a bug? If it was a bug, do you need assistance with vulnerability disclosure and getting a CVE number assigned?

@Marcono1234
Copy link
Contributor

If I understand it correctly, that was only a bug without any security impact.

Zip4j treats an entry with the name / as being the directory to which the files are extracted (so this normally has no effect because that directory already exists). However, the issue was that for the Zip Slip check after resolving the / name the results were:

  • outputCanonicalPath: extract-dir/
  • outputFile.getCanonicalPath(): extract-dir

So the Zip Slip check performing startsWith was erroneously detecting this as Zip Slip attack.
After the fix it is now extract-dir/ in both cases, so startsWith passes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants