Zip Slip Vulnerability | Hacker News Reader

Zip Slip Vulnerability

snyk.io

94 pointszspitzer35 comments

What? You don't get to name this vulnerability. It's one of the oldest in the appsec book.

This is a little bit like trying to brand file upload vulnerabilities that put ..'s in the path name in multipart MIME headers. Wait until they find out about NUL bytes!

Yeah Snyk is pretty well known for bullshit self promotion and terrible articles. I avoid their tool with a passion due to this.

Could you share more about their tool?

Ha..indeed. We wrote something up about it in 2009: https://labs.neohapsis.com/2009/04/21/directory-traversal-in.... But even in that post we note that it was a relatively well known issue.

From the first paragraph:

> Of course, this type of vulnerability has existed before, but recently it has manifested itself in a much larger number of projects and libraries.

They've responsibly notified and helped a number of projects fix this issue[1]. It seems they are being nothing but a good citizen here.

[1] https://github.com/snyk/zip-slip-vulnerability

Every appsec tester looks for these vulnerabilities and routinely finds them. I'm not criticizing them for testing for it; of course they should do that. I'm saying: under no circumstances should they be allowed to brand this vulnerability. It's not "Zip Slip". It's "ZIP file directory traversal".

Yup, this is nothing new. I read a guide on this, and made a PoC tar trojan using this technique back in the '90s.

I'm pretty sure this has been known for a long time. This was the method used to jailbreak some Windows Phones back in the day -- extract an archive that overwrites carrier OMA files. Iirc you could just rename a folder in 7-zip to "../../foo" and it would work. No special tools required.

This class of vulnerabilites (directory traversal in an unarchiver) has been known at least since 2001:

https://nvd.nist.gov/vuln/detail/CVE-2001-1268 (Info-ZIP unzip)

https://nvd.nist.gov/vuln/detail/CVE-2001-1270 (PKZip)

https://nvd.nist.gov/vuln/detail/CVE-2001-1271 (rar)

> I'm pretty sure this has been known for a long time.

It has, but that so many libraries are still vulnerable by default means it hasn't exactly stuck.

Their validation for Node isn't quite correct. They suggest:

  var filePath = path.join(targetFolder, entry.path);
  if (filePath.indexOf(targetFolder) != 0) {
    return;
  }

But if targetFolder="/var/foo", then you can deposit files in "/var/foo.secret" by providing a path "../foo.secret/blub"

This exploit has been found and exploited before (in 2015!) to get root on Samsung devices that ran SwiftKey, that automatically updated on boot as root user through downloading a ZIP from an insecure (HTTP) URL...

See https://www.cvedetails.com/cve/CVE-2015-4641/

If you want to test your favorite unarchiver, but you're too lazy to craft archives by hand:

https://github.com/jwilk/path-traversal-samples

i'm surprised they need to be handcrafted. the zip that ships with OSX will let you create such 'bad' archives. tar will also let you create 'bad' archives. and interestingly enough tar on OSX will barf if you try to extract any of these 'bad' archives. also, there is another trick you can do with tar archives which is not possible for zip archives which is embed a symlink into the archive that points outside the archive and trick the tar program into following the symlink. the tar that ships with OSX will barf on this (even if it wasn't responsible for creating the symlink) but i know on other platforms tar will happily extract through symlinks. i know because there was one PaaS platform where you could extract though symlinks to break out of their sandbox. also, zip will extract through symlinks because it doesn't really understand symlinks. EDIT: (zip understands symlinks. the default OSX version will defer making the symlinks until other files are unzipped which fixes symlinks being used from escaping)

    > zip my.zip ../foo.txt
      adding: ../foo.txt (stored 0%)

    > unzip -l my.zip
    Archive:  my.zip
      Length      Date    Time    Name
    ---------  ---------- -----   ----
            4  06-05-2018 15:08   ../foo.txt
    ---------                     -------
            4                     1 file

    > unzip my.zip
    Archive:  my.zip
    warning:  skipped "../" path component(s) in ../foo.txt
     extracting: foo.txt

    > zip my.zip outside/foo.txt
      adding: outside/foo.txt (stored 0%)
    > rm outside/foo.txt
    > unzip my.zip
    Archive:  my.zip
     extracting: outside/foo.txt
    > cat ../foo.txt
    foo

    > tar cvf my_tar ../foo.txt
    a ../foo.txt

    > tar tvf my_tar.tar
    -rw-r--r--  0 benmurphy wheel       4  5 Jun 15:00 ../foo.txt

    > tar xvf my_tar.tar
    x ../foo.txt: Path contains '..'
    tar: Error exit delayed from previous errors.

    > ln -s ../ outside
    > tar cvf my_tar.tar outside outside/foo.txt
    a outside
    a outside/foo.txt

    > tar tvf my_tar.tar
    lrwxr-xr-x  0 benmurphy wheel       0  5 Jun 15:01 outside -> ../
    -rw-r--r--  0 benmurphy wheel       4  5 Jun 15:02 outside/foo.txt

    > tar xvf my_tar.tar
    x outside
    x outside/foo.txt: Cannot extract through symlink outside/foo.txt
    tar: Error exit delayed from previous errors.

>The contents of this zip file have to be hand crafted.

I think you used to be able to do it on an Amiga using the official LHA(?) compression program, due to the Amiga using front slashes as delimiters.

But beyond that, I really wish there was a zip library that let you do all of the nuanced things the format allows. For example, each file in an archive has its own encryption header, so you could create an archive where the first 10 files were clear text but the 11th was encrypted...

> But beyond that, I really wish there was a zip library that let you do all of the nuanced things the format allows. For example, each file in an archive has its own encryption header, so you could create an archive where the first 10 files were clear text but the 11th was encrypted…

edit: ignore comment, misread encryption as compression. Leaving it for posterity.

That seems pretty straightforward given it's necessary to generate "best practice" OCF. Python's stdlib module certainly allows it (ZipFile.write takes an optional compress_type parameter which overrides the one set in the constructor, just "zf.write(fname, compress_type=zipfile.ZIP_STORED)"

The encryption is a separate step done on top of the compressed data.

Oh yes I misread encryption as compression, sorry.

This is up there with buffer overflow bugs (is counting really that hard?) for things we bitch about all the time, yet somehow developers manage to ignore it.

This type of screwup goes all the way back to DOS days and beyond with similar stunts being attempted with unix tar.

While we're at it, let's repackage people using terrible passwords as a newly discovered widespread problem.

This situation has never shown signs of improvement.

According to their list of known-vulnerable libraries[1], the actual Microsoft-supplied library, System.IO.Compression.ZipFile, either isn't vulnerable or hasn't been tested; only third-party NuGet-based libraries are.

[1]: https://github.com/snyk/zip-slip-vulnerability

Can someone give an example of valid usecase for having `..` at all in a ZIP file?

I would imagine it has less to do with valid use cases and more to do with avoiding special cases and path name validation in your archiver. Surely if you're concerned about filesystem integrity when extracting untrusted archives you're extracting into a chroot jail or similar. Or you're using facilities of the archiver to list the files that would be extracted and validating the list before running the archiver.

Wow, a code name and a logo for behavior that is basically by design?

I've seen this in tar - isn't it a feature?

In tar it definitely is because you can have symlinks using relative paths. So if your directory tree contains symlinks, you tar it with all bells and whistles (attributes, symlinks, ownership) and unpack it somewhere else it won't be a lossy transfer.

Not a good feature. Figuring out the rules that would allow the use of .. in a path name in a safe way seems a lot more difficult than figuring out why you want to put .. into your archive and changing what you are doing to avoid it.

> In fact, Python libraries were vulnerable until fixed in 2014.

This is incorrect. Python's tarfile and zipfile (and shutil) modules have no protection against directory traversal.

https://docs.python.org/3/library/zipfile.html?highlight=zip...

> If a member filename is an absolute path, a drive/UNC sharepoint and leading (back)slashes will be stripped, e.g.: ///foo/bar becomes foo/bar on Unix, and C:\foo\bar becomes foo\bar on Windows. And all ".." components in a member filename will be removed, e.g.: ../../foo../../ba..r becomes foo../ba..r. On Windows illegal characters (:, <, >, |, ", ?, and *) replaced by underscore (_).

Sure reads like directory traversal mitigation.

It does. However, in your linked document just below the `extract` function there is `extractall` that actually has a warning that it does /not/ mitigate directory traversal.

While by itself this might be a deliberate design choice, it vows for an easy mistake.

I'm a fan of telling that a function is unsafe (by design) by its name. So `unsafeExtractall` or something like it. That should give a clear warning to investigate unless you know what you're doing.

`extractall` does mitigate directory traversal. Looking at the source code, both `extract` and `extractall` call `_extract_member` which sanitizes the file paths.

See https://github.com/python/cpython/blob/3.6/Lib/zipfile.py

Good catch. Seems like the docs warn too much on this then, but good to see that it's not a problem in Python.

> It does. However, in your linked document just below the `extract` function there is `extractall` that actually has a warning that it does /not/ mitigate directory traversal.

You're misreading the warning. It's saying that the module attempts to mitigate the issue (referring back to ZipFile.extract) but pointing out that this may not be entirely reliable, and thus that as the library user you really should inspect/validate archive paths before doing an on-disk extraction.

The warning says that it attempts to mitigate it (with a referral to the documentation of the extract function). It's of course a bit strange that in one case it's a note that explains how the mitigation works, and in the other case it's a warning to not blindly trust foreign zips, but it seems clear to me that either both are affected, or neither of them is.

Documentation, the other "hard" task in CS.

I stand corrected. I was mislead by the scary warning, which is still there.

The tarfile module is still vulnerable:

https://bugs.python.org/issue17102

Reply on news.ycombinator.com