Bug 1466872: Fix zip slip vulnerability #99
Conversation
| @@ -150,6 +151,11 @@ func Unzip(b []byte, dest string) error { | |||
|
|
|||
| path := filepath.Join(dest, f.Name) | |||
|
|
|||
| // Fix for https://snyk.io/research/zip-slip-vulnerability | |||
| if !strings.HasPrefix(path, dest) { | |||
There was a problem hiding this comment.
This will not work if dest has not been cleaned (see filepath.Clean).
Also the Unzip function you are patching here is the one that is used to extract zips which are embedded into the worker type definition, which are trusted. Here we are not vulnerable, since if you can modify the content of the zip, you can also modify the absolute location it should be installed to, since both are specified in the same section of the worker type definition, with the same access controls required to modify them.
The code that extracts untrusted zips is in mounts.go which calls github.com/mholt/archiver package. That was already patched in mholt/archiver#65 and since we don't vendor that library, all new builds should pick up the fix. So we should already be safe.
Probably the best is for us to delete this Unzip method, and use package github.com/mholt/archiver everywhere, for simplicity, but again, it seems we are not vulnerable as things stand, so this is just a nice simplification rather than a security enhancement.
Despite this, many thanks for the patch. 😄
Ref: https://snyk.io/research/zip-slip-vulnerability