decompress icon indicating copy to clipboard operation
decompress copied to clipboard

No symlinks

Open jeffrafter opened this issue 4 years ago • 2 comments

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

jeffrafter avatar Feb 27 '20 16:02 jeffrafter

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.

trptcolin avatar Feb 27 '20 18:02 trptcolin

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

jeffrafter avatar Feb 27 '20 19:02 jeffrafter