fixed path traversal issue

[sr1ch1]

This branch contains a fix for a path traversal issue that was reported on the decompress repository see: kevva/decompress#76 but it originates in this repository.
The issues was that path the segment "../" was not filtered out and thus opened a way to place a file outside the location where files should be decompressed.
The fix simply removes any path segments that contain the sequence "../" or "..\".
A test has been added that uses a prepared tar file to show that the issue has been fixed.

[sr1ch1]

It seems that Node.js 4 and 6 are simply too old. Node.js 8 and xo don't work in this combination. The changes I made are not the reason why the build breaks. The last change is from July 2017. So I guess the whole project needs a refresh. I just wanted to provide minimal code changes to make it clear what the fix does. @kevva could you please have a look at this fix.
If you have questions or need support, I am willing to help.

[sr1ch1]

Other issues rely on this one to be fixed:
kevva/download#189
serverless/serverless#7406
serverless/serverless#7402
gatsbyjs/gatsby#21791

[pinxue]

It is a bit overkilling by simply remove "..", it may cause unexpected file overriding, too.
It is better to examine the path will go beyond the starting dir or not, refuse by default and use an option to allow jumping out the jail explicitly.

[sr1ch1]

Maybe this is overkill but on the other hand @jdillick pointed out that other tools prevent this scenario too and one would not expect to find a parent path segment. see kevva/decompress#71 (comment)

[trptcolin]

I think the fix belongs in the main project https://github.com/kevva/decompress

This issue doesn’t just affect tarfiles, but archives in general (including zip, where the zip-slip vulnerability gets its name). So similar fixes would be needed to decompress-unzip, etc. - or the check could be centralized.

I have an alternative PR open in kevva/decompress#73 - lots of interesting comments there that also show that paths containing “..” are not the only way this issue can be exploited. e.g. include a symlink to an absolute path, and write data through it to create/modify a file outside the output directory.

[sr1ch1]

Regarding the symlink issue I agree. Your solution looks like the more thorough fix. I was concerned about possible other dependents. But having a second look I see that the only significant package that uses this package also uses the decompress package.

[dargmuesli]

Now that @trptcolin's PR is merged, how do we get rid of the secvuln notice for this repository?

[heidemn-faro]

I think the fix belongs in the main project https://github.com/kevva/decompress

This issue doesn’t just affect tarfiles, but archives in general (including zip, where the zip-slip vulnerability gets its name). So similar fixes would be needed to decompress-unzip, etc. - or the check could be centralized.

I have an alternative PR open in kevva/decompress#73 - lots of interesting comments there that also show that paths containing “..” are not the only way this issue can be exploited. e.g. include a symlink to an absolute path, and write data through it to create/modify a file outside the output directory.

@trptcolin @sr1ch1 unfortunately this leaves decompress-tar vulnerable, and CVE scanners report it, e.g. Mend or Snyk: https://security.snyk.io/package/npm/decompress-tar/4.1.1
Duplicating the fix seems a better option than leaving the CVE open.

[trptcolin]

FWIW I don’t own this project, don’t have merge/deploy permissions, etc. - just happened to see a vulnerability scanner tag a project I was using a few years ago and did some work on the linked fix. Good luck!

[trptcolin]

Looks like there was a comment 3 years ago from the person who ended up having permissions that someone tagged them on Twitter: kevva/decompress#73 (comment)

[sr1ch1]

I also don't have access. And the person that merged the fix said this package should be considered deprecated :-(