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

[Security] Vulnerability in tar #2625

Closed
asbjornh opened this issue Apr 11, 2019 · 40 comments
Closed

[Security] Vulnerability in tar #2625

asbjornh opened this issue Apr 11, 2019 · 40 comments

Comments

@asbjornh
Copy link

@asbjornh asbjornh commented Apr 11, 2019

Do not open a PR. We appreciate the enthusiasm but the fix is more complicated than it appears. We're considering our options.

See https://www.npmjs.com/advisories/803

Versions of node-tar prior to 4.4.2 are vulnerable to Arbitrary File Overwrite. Extracting tarballs containing a hardlink to a file that already exists in the system, and a file that matches the hardlink will overwrite the system's file with the contents of the extracted file.

Caused by node-gyp. I guess this depends on nodejs/node-gyp#1714 being fixed first. As far as I can tell, to fix this node-sass needs to to upgrade to node-gyp@4.x.x once they've resolved the issue on their part.

Output from yarn audit:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.4.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ node-sass                                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ node-sass > node-gyp > tar                                   │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/803                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
1 vulnerabilities found - Packages audited: 16503
Severity: 1 High
@mohsenari
Copy link

@mohsenari mohsenari commented Apr 11, 2019

Dealing with the same issue. Tried npm update node-sass --depth 999, npm i tar --save, and npm update tar --depth 999. none of that helped updating tar for node-sass

@mohsenari
Copy link

@mohsenari mohsenari commented Apr 11, 2019

Anyone who's looking for a temporary workaround until this gets fixed, I managed to update tar version using izogfif's answer here: https://stackoverflow.com/questions/15806152/how-do-i-override-nested-npm-dependency-versions

You need to remove tar from required section in node-gyp in package-lock.json

Then replace the version in the dependencies section in the same place and remove resolved and integrity properties from tar in dependencies:

"node-gyp": {
		"version": "3.8.0",
		"resolved": "https://registry.npmjs.org/node-gyp/-/node-gyp-3.8.0.tgz",
		"integrity": "sha512-3g8l...",
		"requires": {
			"fstream": "^1.0.0",
			"glob": "^7.0.3",
			"graceful-fs": "^4.1.2",
			"mkdirp": "^0.5.0",
			"nopt": "2 || 3",
			"npmlog": "0 || 1 || 2 || 3 || 4",
			"osenv": "0",
			"request": "^2.87.0",
			"rimraf": "2",
			"semver": "~5.3.0",
			"which": "1"
		},
		"dependencies": {
			"semver": {
				"version": "5.3.0",
				"resolved": "https://registry.npmjs.org/semver/-/semver-5.3.0.tgz",
				"integrity": "sha1-myzl..."
			},
			"tar": {
				"version": "^4.4.2"
			}
		}
},

Then delete your node_modules and run npm i
Test it with npm audit

@meszaros-lajos-gyorgy
Copy link

@meszaros-lajos-gyorgy meszaros-lajos-gyorgy commented Apr 11, 2019

node-gyp updated it's tar version to the latest in this commit a few minutes ago, expecting a release soon:
nodejs/node-gyp@1456ef2

@angelocapone

This comment was marked as off-topic.

@thealiano

This comment was marked as off-topic.

@JarriddW

This comment was marked as off-topic.

@asbjornh
Copy link
Author

@asbjornh asbjornh commented Apr 12, 2019

Same issue for me although I have the 4.4.8 version:
$ npm show tar version
4.4.8

You might have several transitive dependencies on multiple versions of tar :)

@angelocapone

This comment was marked as off-topic.

@JarriddW

This comment was marked as off-topic.

@HarisSpahija

This comment was marked as off-topic.

@JarriddW

This comment was marked as off-topic.

@osushi-desushi
Copy link

@osushi-desushi osushi-desushi commented Apr 12, 2019

package-lock.json

"version": "2.2.1",
"resolved": "https://registry.npmjs.org/tar/-/tar-2.2.1.tgz",
"integrity": "sha1-jk0qJWwOIYXGsYrWlK7JaLg8sdE=",

"version": "4.4.8",
"resolved": "https://registry.npmjs.org/tar/-/tar-4.4.8.tgz",
"integrity": "sha512-LzHF64s5chPQQS0IYBn9IN5h3i98c12bo4NCO7e0sGM2llXQ3p2FGC5sdENN4cTW48O915Sh+x+EXx7XW96xYQ==",


rm -fr node_modules

npm i

npm audit

=== npm audit security report ===

found 0 vulnerabilities
 in 42617 scanned packages

@JarriddW
Copy link

@JarriddW JarriddW commented Apr 12, 2019

@osushi-desushi Your solution has worked, no vulnerabilities. Thanks a ton!

@osushi-desushi

This comment was marked as off-topic.

@meszaros-lajos-gyorgy
Copy link

@meszaros-lajos-gyorgy meszaros-lajos-gyorgy commented Apr 12, 2019

node-gyp got stuck with their part in updating, since they used tar@3 in their repo and upgrading to 4 broke their code: nodejs/node-gyp#1713 (comment)

@clshortfuse
Copy link

@clshortfuse clshortfuse commented Apr 12, 2019

Watch this space:

nodejs/node-gyp#1718

Once node-gyp 3.8.1 comes out, node-sass can update the dependency.

sfentress added a commit to concord-consortium/geocode that referenced this issue Apr 12, 2019
Fixing last high-severity warning, modifying the package-lock file by
hand as per sass/node-sass#2625
@johnDowee

This comment was marked as off-topic.

@prathusingh

This comment was marked as off-topic.

@clshortfuse

This comment was marked as off-topic.

ShahanaFarooqui added a commit to Ride-The-Lightning/RTL that referenced this issue Apr 14, 2019
Manually removed vulnerability by upgrading 'tar' package from 2.2.1 to 4.4.8 (https://stackoverflow.com/questions/55635378/angular-devkit-build-angular-arbitrary-file-overwrite). angular-devkit and node-sass issues are still open. (angular/angular-cli#14138, sass/node-sass#2625). Will permanently be fixed once above 2 issues are addressed by Angular and node-sass teams.
@lenymo

This comment was marked as off-topic.

@sass sass deleted a comment from sotayamashita Apr 15, 2019
@nschonni
Copy link
Contributor

@nschonni nschonni commented Apr 15, 2019

If node-gyp releases a 3.8.1 (or 3.9) there will be no need for a node-sass release as that is in the version range in the package.json already.
Tar is used by node-gyp to download headers for compiling binaries, so this is only an issue if someone gets a malicious tarball on the official nodejs release site and you aren't using our pre-built binaries.

@C-odes

This comment was marked as off-topic.

@HarisSpahija

This comment was marked as off-topic.

@invisor

This comment was marked as off-topic.

@HarisSpahija

This comment was marked as off-topic.

@invisor

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@invisor

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@invisor

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@C-odes

This comment was marked as off-topic.

@nschonni
Copy link
Contributor

@nschonni nschonni commented Apr 15, 2019

Locking the thread, since this is going off topic

@sass sass locked as too heated and limited conversation to collaborators Apr 15, 2019
@nschonni nschonni pinned this issue Apr 23, 2019
@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 23, 2019

For those following along. There's a lot of moving pieces that slowing down resolving this issue.

As stated my @meszaros-lajos-gyorgy in #2625 (comment) the node-gyp are currently blocked from creating a patch because they're using an older version of node-tar to maintain support for older Node versions. Updating node-tar to address this vulnerability would mean breaking support for older versions on Node.

There is a good summary of the node-gyp issue in nodejs/node-gyp#1718 (comment).

The ideal solution would be to patch the version of node-tar being used by node-gyp. There's an issue tracking that request at npm/node-tar#212.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 24, 2019

Please stop opening PRs. We know node-gyp has a new release. That's not enough for our needs. We appreciate the enthusiasm and we're considering our options.

@xzyfer
Copy link
Contributor

@xzyfer xzyfer commented May 15, 2019

A new version of node-tar@2.x has been released with the security patch back ported from 3.x. the security advisory will be updated in the next 24hrs at which npm audit --fix will pass.

npm/node-tar#212 (comment)

@nschonni
Copy link
Contributor

@nschonni nschonni commented May 15, 2019

Advisory has been updated and npm audit fix should work again

@nschonni nschonni closed this May 15, 2019
CristianDavidCabrera added a commit to CristianDavidCabrera/modulo-front-end-avanzado that referenced this issue May 15, 2019
falgon added a commit to falgon/roki that referenced this issue May 25, 2019
@nschonni nschonni unpinned this issue May 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.