Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remediate zipslip #1817

Closed
wants to merge 1 commit into from
Closed

Remediate zipslip #1817

wants to merge 1 commit into from

Conversation

rminnich
Copy link
Member

paths in the archives containing .. could creep out of directory
they were being unpacked into.

The fix is simple: given a base path and file path, instead of
filepath.Join(base, file)
do
filepath.Join(base, filepath.Join("/", file))

Joining to / has the property of swallowing up and .. that may appear.

Signed-off-by: Ronald G Minnich rminnich@gmail.com

paths in the archives containing .. could creep out of directory
they were being unpacked into.

The fix is simple: given a base path and file path, instead of
filepath.Join(base, file)
do
filepath.Join(base, filepath.Join("/", file))

Joining to / has the property of swallowing up and .. that may appear.

Signed-off-by: Ronald G Minnich <rminnich@gmail.com>
@rminnich
Copy link
Member Author

I did file an issue with golang.org and everyone said "it's not a problem" so ... we fix it here.

@orangecms
Copy link
Member

So in other words, this fixes a path traversal flaw?

Could we add some fixtures to test that? Like https://github.com/jwilk/traversal-archives

@ggkitsas
Copy link

ggkitsas commented Sep 1, 2020

So in other words, this fixes a path traversal flaw?

Could we add some fixtures to test that? Like https://github.com/jwilk/traversal-archives

Just a note, this repo will work great for zip and tar, but not for cpio for your case. I needed to create a newc cpio archive:

mkdir -m 755 tmp
umask 022 && echo moo > moo
cd tmp && echo ../moo | cpio -R root:root -H newc  -o > ../moo.cpio
rm -rf tmp moo

@ggkitsas
Copy link

ggkitsas commented Sep 1, 2020

So in other words, this fixes a path traversal flaw?
Could we add some fixtures to test that? Like https://github.com/jwilk/traversal-archives

Just a note, this repo will work great for zip and tar, but not for cpio for your case. I needed to create a newc cpio archive:

mkdir -m 755 tmp
umask 022 && echo moo > moo
cd tmp && echo ../moo | cpio -R root:root -H newc  -o > ../moo.cpio
rm -rf tmp moo

Second note :)
For cpio specifically, you would want to additionally test against symlink-based path traversals (again in newc format)

@rminnich
Copy link
Member Author

ETIMEDOUT

@rminnich rminnich closed this May 25, 2021
@mpictor
Copy link
Contributor

mpictor commented Jul 28, 2021

One way to test this would be to use go-fuzz (or other fuzzer) to create arbitrary cpio inputs, feed em through the extract logic and check for escaping paths.

@insomniacslk
Copy link
Contributor

@rminnich should this be just matter of using upath.SafeFilepathJoin in uzip ?

@rjoleary
Copy link
Contributor

Let's revive this. We're getting security alerts.

@rminnich rminnich deleted the zipslip branch March 8, 2022 23:05
@rjoleary
Copy link
Contributor

Fixed in #2344

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants