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

No symlinks #75

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

No symlinks #75

wants to merge 5 commits into from

Conversation

jeffrafter
Copy link

This addresses #71 by creating a new option symlinks which defaults true. If false then symlinks will not be created at all. This does not match the behavior of unzip or tar which would create the symlink but fail when creating a file outside of the extraction directory with a checkdir error:

$ unzip slip.zip 
Archive:  slip.zip
    linking: symlink_to_root         -> / 
    linking: generic_dir/symlink_to_parent_dir  -> ../ 
checkdir error:  generic_dir/symlink_to_parent_dir exists but is not directory
                 unable to process generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/symlink_to_root/tmp/slipped_zip.txt.
finishing deferred symbolic links:
  symlink_to_root        -> /
  generic_dir/symlink_to_parent_dir -> ../

In this pull request, the symlink is never written at all:

Running:

decompress('slip.zip', 'dist', {symlinks: false}).then(files => {
	console.log('done!');
});

Will not error out but will not create any of the links or /tmp/slipped_zip.txt (though it will create a normal file generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/generic_dir/symlink_to_parent_dir/symlink_to_root/tmp/slipped_zip.txt in the destination folder dist).

This cherry-picks the test fixes from @trptcolin

This fix should be entirely backward compatible but can be leveraged by bin-build and others to ensure that they are not vulnerable in a new version which disables symlinks.

Paired with @goodgravy

@trptcolin
Copy link
Contributor

Does this handle the case of a file path containing .. to traverse outside the directory? You can check w/ this file from my branch - it'll write to ../../../decompress-traversal.txt w/ the released version of decompress.

@jeffrafter
Copy link
Author

@trptcolin I think you are right... I think we might need both

@trptcolin trptcolin mentioned this pull request Mar 3, 2020
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.

3 participants