
Zip Slip Vulnerability - zspitzer
https://snyk.io/research/zip-slip-vulnerability
======
tptacek
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!

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

~~~
throwaway111143
Could you share more about their tool?

------
jaxbot
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.

~~~
jwilk
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](https://nvd.nist.gov/vuln/detail/CVE-2001-1268)
(Info-ZIP unzip)

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

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

------
tlb
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"

------
diamondo25
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/](https://www.cvedetails.com/cve/CVE-2015-4641/)

------
jwilk
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](https://github.com/jwilk/path-traversal-samples)

------
benmmurphy
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.

------
13of40
>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...

~~~
masklinn
> 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)"

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

~~~
masklinn
Oh yes I misread encryption as compression, sorry.

------
evilDagmar
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.

------
nathanaldensr
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](https://github.com/snyk/zip-slip-vulnerability)

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

~~~
jschwartzi
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.

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

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

~~~
the8472
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.

------
jwilk
> 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.

~~~
masklinn
[https://docs.python.org/3/library/zipfile.html?highlight=zip...](https://docs.python.org/3/library/zipfile.html?highlight=zipfile#zipfile.ZipFile.extract)

> 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.

~~~
bjpbakker
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.

~~~
ishi
`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](https://github.com/python/cpython/blob/3.6/Lib/zipfile.py)

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

