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

Vulnerability in downstream library: gorilla/websocket #1091

Closed
DarthHater opened this issue Apr 13, 2020 · 10 comments
Closed

Vulnerability in downstream library: gorilla/websocket #1091

DarthHater opened this issue Apr 13, 2020 · 10 comments

Comments

@DarthHater
Copy link

Hi there!

I work on the team that writes nancy, and in a scan of a project where I use cobra, I discovered that your downstream dependency on gorilla/websocket is effected by: GHSA-jf24-p9p9-4rjh

Not sure if this would make cobra vulnerable, but I figured I'd file an issue, as upgrading to 1.4.1 seems trivial and gets you out of the line of fire :)

Cheers,
Jeffry

@jpmcb
Copy link
Collaborator

jpmcb commented Apr 21, 2020

Thanks for marking this issue! I've looked into this and it appears that gorilla/websocket is a dependency of spf13/viper:

$ go mod graph
...
github.com/spf13/viper@v1.6.3 github.com/gorilla/websocket@v1.4.0

but is not a indirect dependency for cobra itself:

$ go mod why -m github.com/gorilla/websocket
# github.com/gorilla/websocket
(main module does not need module github.com/gorilla/websocket)

and therefore does not appear in our go.mod file.

I believe we can close this issue and I'll go ahead and raise the issue with the viper repo!

@DarthHater
Copy link
Author

Perfecto! Thanks a ton @jpmcb . If you or others are into it I can send you a PR with nancy so that y'all can get alerted when these happen?

@DarthHater
Copy link
Author

Now that viper/etc... are updated, do you plan to update to the newest viper? I imagine we can do that on our end, but if cobra by default has it on a newer version, that's nice :)

@jpmcb
Copy link
Collaborator

jpmcb commented May 1, 2020

Sorry for the delay on this!

We have been investigating using Dependabot to automatically update these dependencies. So hopefully in the future, we can catch these quickly!

But for now, it looks like viper hasn't released the changes I made to the Gorilla dependency. And it appears that it remains an indirect dependency, unused by cobra. So I'm not sure if a replace in the go.mod of this project is necessary. I've seen Go mod project's dependencies get rather unwieldy when downstream they attempt to update nested dependencies (in this case, a dependency that viper depends on). I agree that we should update as soon as possible but not at the expense of making the go mod difficult to maintain.

In the mean time, a workaround for your upstream project may be to add this to your go.mod:

replace github.com/gorilla/websocket => github.com/gorilla/websocket v1.4.2

@DarthHater
Copy link
Author

It might show I'm a bit green with the replace functionality, but I had thought that was for only using local forks?

As well:

https://github.com/sonatype-nexus-community/nancy

I work on that project and it can be used in CI/CD to report on vulns in the libraries you are using. Automatically updating dependencies gives me a bit of panic (personally), because you have a pretty high trust level in what you consume, if you wanted to look at an alternative (mostly just to know if you have issues), Nancy might be worth looking at (it's how I found this upstream issue :) )

@jpmcb
Copy link
Collaborator

jpmcb commented May 1, 2020

At the end of your go.mod file, you can add a replace statement for different versions of a package. You can also do this to replace packages with local "vendors" or forks (like if you were working on a new feature locally and wanted to try it out)

So, for example, here's a simple test app generated with the cobra cli:

package main

import "testing-app/cmd"

func main() {
	cmd.Execute()
}

here's the go.mod:

module testing-app

go 1.14

require (
	github.com/mitchellh/go-homedir v1.1.0
	github.com/spf13/cobra v0.0.7
	github.com/spf13/viper v1.6.2
)

replace github.com/gorilla/websocket => github.com/gorilla/websocket v1.4.2

and here's the resulting go.sum:

$ cat go.sum | grep gorilla
github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=

So you can see that the gorilla package is replaced in the go sum file correctly with version 1.4.2. I would also advise caution doing this since you are effectively bumping a dependency that a downstream package depends on. You may end up with unexpected behavior.

@umarcor
Copy link
Contributor

umarcor commented May 11, 2020

FTR, #1012 is an open PR to update viper, pflag and yaml.

We have been investigating using Dependabot to automatically update these dependencies. So hopefully in the future, we can catch these quickly!

@jpmcb, that'd be really nice! That's how I keep my fork of cobra up to date.

ptrus added a commit to oasisprotocol/tendermint that referenced this issue May 20, 2020
* github.com/spf13/viper@v1.5.0 -> github.com/spf13/viper@v1.7.0

* replace directive to update the version as spf13/cobra doesn't have a
release with the fix yet: spf13/cobra#1091
ptrus added a commit to oasisprotocol/tendermint that referenced this issue May 20, 2020
* github.com/spf13/viper@v1.5.0 -> github.com/spf13/viper@v1.7.0

* replace directive to update the version as spf13/cobra doesn't have a
release with the fix yet: spf13/cobra#1091
@github-actions
Copy link

This issue is being marked as stale due to a long period of inactivity

@jpmcb
Copy link
Collaborator

jpmcb commented Jul 13, 2020

This has been updated in #1012 and the dependency graph now shows:

$ go mod graph
...
github.com/spf13/viper@v1.7.0 github.com/gorilla/websocket@v1.4.2

Thanks so much for raising the issue and letting the maintainers know about this security vulnerability!

@jpmcb jpmcb closed this as completed Jul 13, 2020
@DarthHater
Copy link
Author

@jpmcb not a problem, glad to help!

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

4 participants