TritonDataCenter / node-http-signature

Reference implementation of Joyent's HTTP Signature Scheme
https://tritondatacenter.com
MIT License
405 stars 118 forks source link

snyk security vuln. - update sshpk version #68

Closed knoxcard closed 4 years ago

knoxcard commented 6 years ago

✗ High severity vulnerability found on sshpk@1.13.1

lc3t35 commented 6 years ago

+1

austinkelleher commented 6 years ago

Can we please get this merged @arekinath @davidlehn? Thanks in advance.

davidlehn commented 6 years ago

@austinkelleher I'm not a maintainer here, just a former contributor and user. Note that in this case it's likely a semver based install will use the newer sshpk version anyway.

wyardley commented 6 years ago

FWIW, even if I remove node_modules and reinstall, I get an nsp warning for https://nodesecurity.io/advisories/606, e.g., opsgenie-sdk@0.4.2 > requestretry@1.13.0 > request@2.83.0 > http-signature@1.2.0 > sshpk@1.13.1 or @google-cloud/pubsub@0.18.0 > google-gax@0.16.1 > grpc@1.8.4 > node-pre-gyp@0.6.39 > request@2.83.0 > http-signature@1.2.0 > sshpk@1.13.1

NejcZdovc commented 6 years ago

@wyardley security problem was fixed with 1.14.1, where you have version 1.13.1 installed

knoxcard commented 6 years ago

GitHub needs a community pull voting system that does not require permission of a maintainer. This project has 24 contributors, but appears to only have one maintainer @arekinath? Not good. Democracy should rule all, looks like I need to submit a pull request under GitHub to enable? :-)

benwiggins commented 6 years ago

Or you could simply not create unnecessary pull requests.

You're pulling in an old, out of date dependency, most likely caused by your own package lock file.

There is a hint right there in your snyk output. When you see something like

Your dependencies are out of date, otherwise you would be using a newer sshpk than sshpk@1.13.1.

Snyk is telling you that you are pinning sshpk at @1.13.1, and it's not node-http-signature.

knoxcard commented 6 years ago

@benwiggins - haha, I was unaware of this, thanks for educating me about this. Right on.

knoxcard commented 6 years ago

On that note, closing pull request and exiting building...

davidlehn commented 6 years ago

Lock file nonsense aside, this PR is still a good idea to ensure a proper sshpk version is used.

knoxcard commented 6 years ago

What I am gathering so far is that you can solve this issue now by utilizing the snyk package. How about those who are unaware of Snyk or fail to do frequent maintenance checks (snyk wizard)? I'd then tend to agreee with @davidlehn, that this PR still does some good.

Personally, I run snyk wizard twice per day to ensure my system is up to par.

npm install snyk -g 

or

yarn add snyk global

then

snyk wizard

Follow the prompts accordingly to patch your system, updating local dependencies and applying available patches wherever possible. If my general assessment stated here is incorrect or you have any additional helpful recommendations, please share...

benwiggins commented 6 years ago

Don’t get me wrong, I 100% agree that bumping the minimum version to a known “good” version is good practice / courtesy.

But it does nothing to educate those tearing their hair out creating pull requests and issues because they don’t understand how dependency resolution/semvers/package locks actually work.

Nor does it necessarily solve someone’s nsp or snyk “security issue” if your package is a nested(-nested-nested) dependency, which in this case being a request dependency, is pretty likely. It just moves the pull request/issue noise somewhere else :)

kusor commented 4 years ago

Done as of PR #86