Skip to content

Conversation

@nhz2
Copy link
Contributor

@nhz2 nhz2 commented Apr 19, 2024

ZipArchives doesn't currently have a safe way to unzip into a temporary directory.

The write(joinpath(newpath, file), zip_readentry(zr, file, String)) code you are using is very dangerous.

This could allow a bad zip file with a file name of for example "/home/bob/.ssh/bad_secret_key" to overwrite any file in your file system. p7zip_jll has many additional checks to try and prevent this.

If you want to use ZipArchives safely you need to make sure the file names are what you expect them to be before writing any files.

@codecov
Copy link

codecov bot commented Apr 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.06%. Comparing base (2ab96aa) to head (d3736d4).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #140      +/-   ##
==========================================
- Coverage   81.09%   81.06%   -0.04%     
==========================================
  Files          29       29              
  Lines        2116     2107       -9     
==========================================
- Hits         1716     1708       -8     
+ Misses        400      399       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@richardreeve
Copy link
Member

richardreeve commented Apr 19, 2024

Thanks @nhz2 - that's very helpful. It's a shame that ZipArchives doesn't allow direct unzip of a zip archive safely though. I'm always sad when I have to use run() to get something to work because there's no native Julia implementation... anyway, I'll merge this once the checks have passed.

@richardreeve richardreeve self-requested a review April 19, 2024 10:13
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

Successfully merging this pull request may close these issues.

2 participants