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" #1147

Closed
fitzhavey opened this issue Apr 12, 2019 · 11 comments
Closed

Security Vulnerability in "tar" #1147

fitzhavey opened this issue Apr 12, 2019 · 11 comments

Comments

@fitzhavey
Copy link

fitzhavey commented Apr 12, 2019

Current behavior

npm audit fails due to security vulnerabilities in the package tar

#!/bin/bash -eo pipefail
npm audit
=== npm audit security report ===                        
                                                                                
┌──────────────────────────────────────────────────────────────────────────────┐
│                                Manual Review                                 │
│            Some vulnerabilities require your attention to resolve            │
│                                                                              │
│         Visit https://go.npm.me/audit-guide for additional guidance          │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.4.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ semantic-release [dev]                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ semantic-release > @semantic-release/npm > npm > libcipm >   │
│               │ npm-lifecycle > node-gyp > tar                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/803                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.4.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ semantic-release [dev]                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ semantic-release > @semantic-release/npm > npm > libnpm >    │
│               │ npm-lifecycle > node-gyp > tar                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/803                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.4.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ semantic-release [dev]                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ semantic-release > @semantic-release/npm > npm > node-gyp >  │
│               │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/803                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬───��──────────────────────────────────────────────────────────┐
│ High          │ Arbitrary File Overwrite                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ tar                                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=4.4.2                                                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ semantic-release [dev]                                       │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ semantic-release > @semantic-release/npm > npm >             │
│               │ npm-lifecycle > node-gyp > tar                               │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/803                       │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 4 high severity vulnerabilities in 16960 scanned packages
  4 vulnerabilities require manual review. See the full report for details.
Exited with code 1

This is present in version 15.13.3

@pyrho
Copy link

pyrho commented Apr 12, 2019

For anyone feeling they're blocked by this, as stated by a maintainer here on another similar issue, on yarn you can use resolutions to force which package version you want to use (for tar in this case).
{ "resolutions": { "tar": ">=4.4.2" } } does the trick.

See https://yarnpkg.com/lang/en/docs/selective-version-resolutions/

@fitzhavey
Copy link
Author

fitzhavey commented Apr 12, 2019

@pyrho thanks for this. However we aren't in a position to move to yarn as we use a distributed system of microservices. This makes this a totally blocking issue for us 😞

@dominykas
Copy link
Contributor

dominykas commented Apr 12, 2019

Why is this issue on semantic-release anyways? The problem is in a dep of node-gyp, and I believe it's getting backported to all versions? nodejs/node-gyp#1713

@dominykas
Copy link
Contributor

Also the issue itself is likely not even something that affects you the way node-gyp uses it - it's a pity there is no way to silence the alerts in npm audit, but in this case you totally should, unless you don't trust tars of node headers distributed via official repos.

@simlu
Copy link
Contributor

simlu commented Apr 14, 2019

For anyone feeling they're blocked by this, as stated by a maintainer here on another similar issue, on yarn you can use resolutions to force which package version you want to use (for tar in this case).
{ "resolutions": { "tar": ">=4.4.2" } } does the trick.

See https://yarnpkg.com/lang/en/docs/selective-version-resolutions/

I guess it's time to say goodbye to npm in favor of yarn once and for all. Luckily we were pretty far down that road already anyways :)

Edit: Oh boy. Thanks for all the downvotes. See below for explanation before adding more downvotes (I've also quoted the corresponding reply). This was not bashing on npm but rather pointing out a missing feature in npm.

@dominykas
Copy link
Contributor

I guess it's time to say goodbye to npm in favor of yarn once and for all. Luckily we were pretty far down that road already anyways :)

@simlu funny you should say that. If I read the code correctly, yarn uses the exact same version of node-gyp as npm does. Not only that, if you were to npm i -g npm to upgrade npm, it would eventually fix that. Reinstalling yarn won't, because it's not listed in its package.json.

@byCedric
Copy link
Contributor

byCedric commented Apr 24, 2019

Ok, so I did some digging around. This is definitely not the repository that "causes" or "needs to fix" the issue. But please, fighting over "use yarn instead of npm" doesn't help (at all)...

To sum up the difficulties:

There are no simple solutions for this issue. Although it's always a good idea to sum some up.

  1. npm-lifecycle could upgrade to node-gyp@>=4, which was patched with tar@4
  2. tar@2 could get a security patch, but that's kind of an old version already...
  3. Some people say the vulnerability doesn't apply for tar@2, but I have no clue if this is true or not. If it is, I can imagine NPM updating it's audit report to exclude the version.

This security issue has been rough on the community, on all of us. Again, I don't think it helps anything to be negative to each other or letting this escalate further. Luckily for us, the guys/girls from the Node.js Foundation Technical Steering Committee have recognized this issue and should be talking about this today (2019-04-24).

I sincerely hope they can come up with something, else we probably have to wait until NPM and npm-lifecycle upgrades to a newer node-gyp version. Semantic Release can't do much about this I'm afraid.

@simlu
Copy link
Contributor

simlu commented Apr 24, 2019

@dominykas I'm a little lost. I think there is some serious confusion here. Let me try to put this together.

I'm not saying that semantic-release should use yarn instead of npm. I'm saying that (wrt the first response to this ticket), we switching our projects that use semantic-release from using npm for installing the package.json file to using yarn for doing that. The reason being that yarn allows you to forcefully upgrade nested sub-dependencies, but npm doesn't. Does that make sense?

The fact that the vulnerability exists inside a sub-dependency inside semantic-release which happens to be npm, has nothing to do with my reply above.

Apologies if I misunderstand something here. But that's what I concluded... At no point was this meant to be offensive or an argument. I was simply happy to find a fix for the vulnerability and yarn provided it while npm didn't.

@byCedric
Copy link
Contributor

I think I missed your point too, I totally forgot you can do that too... I read some irritations that "hits close to home", wanted to find out once and for all if I could do anything. Sorry if this was a bombshell @simlu, definitely not intended as one!

@dominykas
Copy link
Contributor

Sorry, I probably read that as a snark and responded without stopping to think if it's constructive.

The reason being that yarn allows you to forcefully upgrade nested sub-dependencies, but npm doesn't

It's a very powerful feature. Dangerous too. But I guess that's veering offtopic.

gregswindle added a commit to commonality/archetypes-rules that referenced this issue Apr 27, 2019
This is no fix for the tar vulnerability right now:

semantic-release/semantic-release#1147

Since all audit issues occur with devDependencies only, and
because our distribution will not include any devDependencies,
I'm choosing the ignore audit issues for now.
@pvdlg
Copy link
Member

pvdlg commented Jun 13, 2019

Thanks @byCedric for the detailed explanation.
There is nothing we can do in semantic-release, the problem is in npm which is a dependency of @semantic-release/npm.

@pvdlg pvdlg closed this as completed Jun 13, 2019
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

No branches or pull requests

6 participants