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

Update websocket-extensions to address CVE-2020-7663 #73

Closed
wants to merge 3 commits into from

Conversation

blakewest
Copy link

See the CVE here. The fix is just to bump the version.

@jcoglan
Copy link
Collaborator

jcoglan commented Jun 6, 2020

Hi @blakewest, thank you for opening this but I'm a bit confused about why this change is necessary or how it works, in terms of making sure end users upgrade to a safe version of websocket-extensions. Could you explain a little about what prompted you to make this change?

@blakewest
Copy link
Author

blakewest commented Jun 6, 2020

@jcoglan yeah, I work for a company that uses websocket-driver-ruby. One of the dependencies of that gem (websocket-extensions) has a security vulnerability that was just discovered yesterday (see the CVE link I put in the PR description). We found out about this because we run salus security checks on our CI, and yesterday, those checks started failing (and hence blocking deploys), because of this security issue. We hot fixed by hard coding a change in our Gemfile.lock to implicitly upgrade websocket-extensions to the new patched version (1.15). But patching the Gemfile.lock is only a temp fix. This PR is the real fix, so that we don't have to have our weird temp fix of manually adjusting the Gemfile.lock

I was going off of this blog post, which I now see is actually your blog... so perhaps I am confused. The blog says pretty clearly, "To mitigate this issue, you should install one of these packages as appropriate for your system: version 0.1.5 of the rubygems package. All previous versions of these packages are vulnerable."

Which is why I bumped the version to 0.1.5. Am I missing something?

@jcoglan
Copy link
Collaborator

jcoglan commented Jun 7, 2020

Yes, both this package and https://github.com/faye/websocket-extensions-ruby are part of the Faye organisation, which I'm responsible for. That security issue was reported to, and fixed by, me.

Updating your Gemfile.lock by using the bundle update command is how these situations are supposed to be resolved. It's not necessary for every package in the dependency graph between the affected package and end users to be re-published, because applications should pin the exact versions they're running with a lockfile and subscribe to security alerts for all libraries they're using, not just immediate dependencies.

@blakewest
Copy link
Author

Yeah, that's totally fair. I guess why not just upgrade your dependencies anyway, given that this is a minor version change? If you don't want to, we can close it.

@jcoglan
Copy link
Collaborator

jcoglan commented Jun 7, 2020

I've given a more full answer on the Node.js version of this package, for which there is a similar PR, here: faye/websocket-driver-node#37 (comment). That was motivated by the fact that upgrading transitive dependencies can be harder in Node.js than it is in Ruby.

My core concern here is that security alerts should route directly to affected applications without all intermediate packages having to "bubble up" the patch manually by releasing new versions. We do have mechanisms for achieving that, but if there is a convincing argument that it would improve end-user security, then I'm prepared to go ahead with all the releases that would entail within this organisation.

It would still require hundreds of other packages to make similar changes in order for the change to bubble up manually, which is why I'd rather the default expectation is that security alerts make their way directly to affected applications.

@jcoglan
Copy link
Collaborator

jcoglan commented Mar 12, 2021

I'm going to close this -- I've not had any further feedback anywhere else about people having trouble upgrading these packages, so I don't believe there's anything more to do here.

@jcoglan jcoglan closed this Mar 12, 2021
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

Successfully merging this pull request may close these issues.

2 participants