5 ms·

Zip Slip Vulnerability

94 points by zspitzer 8y ago

deleted 8y ago
[deleted]
13of40 8y ago
>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 8y ago
  > 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 8y ago
    The encryption is a separate step done on top of the compressed data.
    - masklinn 8y ago
      Oh yes I misread encryption as compression, sorry.
jaxbot 8y ago
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.
- masklinn 8y ago
  > 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.
- jwilk 8y ago
  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)
deleted 8y ago
[deleted]
nathanaldensr 8y ago
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
jwilk 8y ago
> 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 8y ago
  https://docs.python.org/3/library/zipfile.html?highlight=zipfile#zipfile.ZipFile.extract 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.
  - bjpbakker 8y ago
    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 8y ago
      `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 8y ago
        Good catch. Seems like the docs warn too much on this then, but good to see that it's not a problem in Python.
dpedu 8y ago
I've seen this in tar - isn't it a feature?
- wang_li 8y ago
  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.
- the8472 8y ago
  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 8y ago
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 8y ago
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.
tptacek 8y ago
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 8y ago
  Yeah Snyk is pretty well known for bullshit self promotion and terrible articles. I avoid their tool with a passion due to this.
  - throwaway111143 8y ago
    Could you share more about their tool?
- oh_sigh 8y ago
  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 https://github.com/snyk/zip-slip-vulnerability
  - tptacek 8y ago
    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".
    - deleted 8y ago
      [deleted]
- ptoomey3 8y ago
  Ha..indeed. We wrote something up about it in 2009: https://labs.neohapsis.com/2009/04/21/directory-traversal-in-archives/ 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.
ComputerGuru 8y ago
Wow, a code name and a logo for behavior that is basically by design?
tlb 8y ago
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"
zokier 8y ago
Can someone give an example of valid usecase for having `..` at all in a ZIP file?
- jschwartzi 8y ago
  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.
diamondo25 8y ago
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/
evilDagmar 8y ago
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.

94 points by zspitzer 8y ago

deleted 8y ago

[deleted]

>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 8y ago
> 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 8y ago
  The encryption is a separate step done on top of the compressed data.
  - masklinn 8y ago
    Oh yes I misread encryption as compression, sorry.

jaxbot 8y ago

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.

masklinn 8y ago
> 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.
jwilk 8y ago
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)

deleted 8y ago

[deleted]

nathanaldensr 8y ago

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

jwilk 8y ago

> 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 8y ago
https://docs.python.org/3/library/zipfile.html?highlight=zipfile#zipfile.ZipFile.extract 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.
- bjpbakker 8y ago
  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 8y ago
    `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 8y ago
      Good catch. Seems like the docs warn too much on this then, but good to see that it's not a problem in Python.

dpedu 8y ago

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

wang_li 8y ago
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.
the8472 8y ago
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 8y ago

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 8y ago

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.

tptacek 8y ago

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 8y ago
Yeah Snyk is pretty well known for bullshit self promotion and terrible articles. I avoid their tool with a passion due to this.
- throwaway111143 8y ago
  Could you share more about their tool?
oh_sigh 8y ago
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 https://github.com/snyk/zip-slip-vulnerability
- tptacek 8y ago
  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".
  - deleted 8y ago
    [deleted]
ptoomey3 8y ago
Ha..indeed. We wrote something up about it in 2009: https://labs.neohapsis.com/2009/04/21/directory-traversal-in-archives/ 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.

ComputerGuru 8y ago

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

tlb 8y ago

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"

zokier 8y ago

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

jschwartzi 8y ago
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.

diamondo25 8y ago

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/

evilDagmar 8y ago

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.