SECURITY: vulnerable to zip-slip (possible remote code execution/file overwrite)

[aviadatsnyk]

Background

https://github.com/snyk/zip-slip-vulnerability

Steps to reproduce

1. get a malicious zip file, i.e. https://gist.github.com/grnd/61e3244bfff85cbe025f5bd4c60ba842
2. unpack with new FastZip().ExtractZip(@"zip-slip-win.zip", @"c:\target", null);
3. profit

Expected behavior

either the whole extraction should be stopped, or the files that are being extracted outside of the destination folder should not be extracted.

Actual behavior

All files are extracted.

Version of SharpZipLib

All

Obtained from (place an x between the brackets for all that apply)

• Compiled from source
• branch: all
• commit: any
• Downloaded DLL from GitHub
• Downloaded DLL from SourceForge
• Downloaded DLL from _______
• DLL included as part of
• Package installed using:
• NuGet
• MyGet
• Chocolatey

Reporters

Yusuke Fujiwara - @yfakariya
Yann McCready, Genetec

[piksel]

I am well aware of "Zip Snip" as it's an exploit as old as the zip format.

The behaviour is currently as intended, although one might argue that disallowing parent directory references in paths and symlinks should be done by default since it prevents (some) misuse.

The root issue is still:
If you do not trust the file source, you should not be blindly extracting it in the first place.

[aviadatsnyk]

Hey @piksel - indeed this is a feature of the zip format, that has security implications.
As you wrote, it is widely accepted by many zip extraction implementation and libraries that this behaviour should be limited, a partial list of them is in snyk/zip-slip-vulnerability and many others are already limiting this behaviour and are therefore not listed there.

Of course, all of these do not mean this library has to have the same view. However, my opinion is that limiting this dangerous feature is the norm, and so any design choice that is significantly different should be clearly communicated to users. Having corresponded with @christophwille - he suggested I open this issue so users are aware of it.

Personally, I would also suggest re-considering the decision to have this as the intended behaviour, since many users do extract zip files from untrusted sources, and some are using this library to do so. In many cases, this vulnerability could lead to remote code execution. A less extreme decision might be to include this either as a warning or a clarification in the README.

[jfreilly]

Although the library in the end can't protect users from themselves it might be a good idea to make some steps to make it harder for them to mangle things. This would probably complicate things internally as the likely thing is to have another setting that by default stops this behaviour. At a minimum testing archives should warn of absolute paths, paths with device/drive names in them and relative paths that point to higher level directories so people who check are provided with suitable information before they extract the files.

…

On Mon, 11 Jun 2018 at 23:23 aviadatsnyk ***@***.***> wrote: Hey @piksel <https://github.com/piksel> - indeed this is a feature of the zip format, that has security implications. As you wrote, it is widely accepted by many zip extraction implementation and libraries that this behaviour should be limited, a partial list of them is in snyk/zip-slip-vulnerability <https://github.com/snyk/zip-slip-vulnerability#affected-libraries> and many others are already limiting this behaviour and are therefore not listed there. Of course, all of these do not mean this library has to have the same view. However, my opinion is that limiting this dangerous feature is the norm, and so any design choice that is significantly different should be clearly communicated to users. Having corresponded with @christophwille <https://github.com/christophwille> - he suggested I open this issue so users are aware of it. Personally, I would also suggest re-considering the decision to have this as the intended behaviour, since many users do extract zip files from untrusted sources, and some are using this library to do so. In many cases, this vulnerability could lead to remote code execution. A less extreme decision might be to include this either as a warning or a clarification in the README. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#232 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAVbBlfTxiVRtqRXg9bILWqn8uI-g5Khks5t7lMhgaJpZM4Uib19> .