zipper icon indicating copy to clipboard operation
zipper copied to clipboard

Zipper might be vulnerable to Zip Slip

Open oranja opened this issue 6 years ago • 7 comments

The details of the vulnerability are detailed in this post: https://snyk.io/research/zip-slip-vulnerability To summarize, Zip files can be crafted to contain entries with paths such as ../../../../../tmp/evil.sh, which point to outside of the extraction folder and might be used to overwrite important executables or config files. Therefore, it's crucial to check on file extraction that the path doesn't escape from the scope of the destination folder.

oranja avatar Jun 05 '18 17:06 oranja

@Lecrapouille Does this mean this package is vulnerable to zipslips rn? if so - worth adding a comment about it to the readme

aviadhahami avatar Sep 13 '21 17:09 aviadhahami

What do you mean by rn ? Zipper by its initial design allows 1/ replacing files when unzipping them (which is not good when you are root) and 2/ allowing relative path name in the zip is also not good since you can extract file outside the folder you want and finally when naming a file ../../../../ ... ../ which a path outer the /the unix system will place at / so now you can make such attack ../../../../ ... ../etc/passwd are replace the original password.

PS: I've reverted my patch on the dev branch that because I finally do not want to break the API, but I forget to restore this issue and the README. I'll stop maintaining this branch and I'll soon make some changes on the branch v2.x.y and change a little the API and add my patch on this branch.

Lecrapouille avatar Sep 13 '21 18:09 Lecrapouille

by rn (="right now", just to be on the safe side), I mean since https://github.com/sebastiandev/zipper/commit/e20fc3ffe6d33827d3ff4fa0e810d6a9e7955101 was merged into master 5 days ago.

aviadhahami avatar Sep 14 '21 10:09 aviadhahami

Can confirm that the latest build is vulnerable to zipslip 🦗 ⚠️

aviadhahami avatar Sep 14 '21 13:09 aviadhahami

@aviadhahami yes I pushed-force the README back since to fix this vulnerability I have to break the behavior or the API and I finally do not want it since I'm not the original author and I cannot talk with them about API changes (since this vulnerability holds 3 years now and minizip is 7 years old ... so no urge). That is why I'll refresh the project on a new branch 2.x.y but for the moment I have little time. Currently, in branch 2.x.y I merged files and one-line functions, rethink the project folder, remove Cmake for a lighter makefile, compiling for Linux. The next step will be to clean bad C++ code: removing new/free and use unique pointers ..., add unit tests (since this a pain for me not breaking behavior with pull requests), and finally change the API and vulnerability. Once stable I'll merge it to dev and then to master branch.

Lecrapouille avatar Sep 14 '21 19:09 Lecrapouille

@Lecrapouille - Ok, sounds good :) In the meantime - can we explicitly state that the package is vulnerable to zipslip? It will allow developers to put mitigations ahead of time :)

I can PR the README if you'd like

aviadhahami avatar Oct 12 '21 11:10 aviadhahami

Yes thanks since currently I've no time

Lecrapouille avatar Oct 12 '21 16:10 Lecrapouille

Added non regression tests in v2.x.y. I'll not fix it on the master API since this will break the API.

Lecrapouille avatar Sep 11 '22 10:09 Lecrapouille