Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Potential security vulnerability via hoek #106

Closed
dbaeumer opened this issue Apr 27, 2018 · 15 comments
Closed

Potential security vulnerability via hoek #106

dbaeumer opened this issue Apr 27, 2018 · 15 comments
Assignees
Milestone

Comments

@dbaeumer
Copy link
Member

See https://github.com/Microsoft/vscode-languageserver-node/network/dependencies

I get this in vscode-languageserver-node which has a dependency on vscode which then via n levels has a dependency on hoek. Do we need to update something?

@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

microsoft/vscode#48783

@bpasero bpasero closed this as completed Apr 27, 2018
@bpasero bpasero reopened this Apr 27, 2018
@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

Actually lets close the other one as duplicate of this one.

@bpasero bpasero self-assigned this Apr 27, 2018
@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

This depends on request/request#2926 which we depend on.

@DanTup
Copy link
Contributor

DanTup commented Apr 27, 2018

Based on this comment it might be gulp-remote-src using an old version of request which results in an old version of hoek. I think the latest version of request depends on a version of hoek that is not vulnerable based on the comments in the related cases.

@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

@DanTup I was assuming the security issue is for any hoek version < 5, which would mean even the latest request version does not fix it?

@DanTup
Copy link
Contributor

DanTup commented Apr 27, 2018

The fix was apparently ported back to the latest v4 release. Some comments at hapijs/hoek#230 (comment) about it.

@DanTup
Copy link
Contributor

DanTup commented Apr 27, 2018

This looks like the fix in v4.2.1: https://github.com/hapijs/hoek/blob/v4.2.1/lib/index.js#L116-L118

@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

@DanTup this needs to bubble to GitHub though that still recommends 5.x:

image

@DanTup
Copy link
Contributor

DanTup commented Apr 27, 2018

Yeah, the CVE page still says that and I guess GH gets its data from there. Once that's fixed, I think gulp-remote-src will still need addressing though. I guess they'll have had an alert the same as us all though!

@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

@DanTup it looks like gulp-remote-src is not compatible with latest request, so either they need to fix it or we have to move to another library for this.

@DanTup
Copy link
Contributor

DanTup commented Apr 27, 2018

Yeah. I've opened ddliu/gulp-remote-src#16 to prompt them; though I suspect they had an alert from GH too.

@bpasero
Copy link
Member

bpasero commented Apr 27, 2018

@DanTup thanks

@gwicksted
Copy link

Just an FYI that hoek 4.2.1 has been deemed not vulnerable (request comment) so it looks like the request group is going with that version to keep node 4 compatibility. Hopefully GitHub will detect that as a safe version instead of only hoek ~> 5.0.3.

@joaomoreno
Copy link
Member

joaomoreno commented May 2, 2018

@egamma pinged me about this, since there are extensions out there flagging up as having a security vulnerability.

This repository also got the warning as soon as I pushed a package-lock.json.

I've created a PR for gulp-remote-src: ddliu/gulp-remote-src#17. Let's hope they take it.

Meanwhile, I've also forked that repo with the fix, published it to npm as gulp-remote-src-vscode and updated vscode to depend on that instead of the official module. I've pushed that change to master and now the vulnerability is gone. If gulp-remote-src takes the change, we can change back our dependency to it.

@bpasero Please review and test the current master. If good, simply publish a new version, so extensions can start fixing their vulnerability warnings.

@bpasero
Copy link
Member

bpasero commented May 2, 2018

Thanks, released 1.1.17 of vscode module.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants