Skip to content

Update github.com/gogo/protobuf to v1.3.2#2478

Merged
simonpasquier merged 1 commit intoprometheus:masterfrom
simonpasquier:bump-gogo-dep
Feb 9, 2021
Merged

Update github.com/gogo/protobuf to v1.3.2#2478
simonpasquier merged 1 commit intoprometheus:masterfrom
simonpasquier:bump-gogo-dep

Conversation

@simonpasquier
Copy link
Member

Fix for CVE-2021-3121

Signed-off-by: Simon Pasquier spasquie@redhat.com

Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The frontend CI test has recently started to fail everywhere (/bin/sh -c npm install -g elm@0.19.0 elm-format@0.8.0 elm-test@0.19.0-beta5 uglify-js@3.4.7 doesn't work anymore).

Perhaps we should do the same (or do it first) in release-0.21 so that we can cut a bugfix release before releasing the more invasive changes like silences with negative matchers (also see #2479 )?

@simonpasquier
Copy link
Member Author

The frontend CI test has recently started to fail everywhere

yes I've noticed too as I was confused why this PR would break anything with the frontend :)

@simonpasquier
Copy link
Member Author

simonpasquier commented Feb 8, 2021

and yes to patch release-0.21 first :)

@simonpasquier simonpasquier force-pushed the bump-gogo-dep branch 5 times, most recently from 995d53c to b3bb2e7 Compare February 8, 2021 17:13
@beorn7
Copy link
Member

beorn7 commented Feb 8, 2021

Is what @roidelapluie said in prometheus/prometheus#8446 also true for this PR? In that case, I'd say we shouldn't put it in a bugfix release.

@simonpasquier
Copy link
Member Author

To be honest, I have a hard time assessing how you can exploit the vulnerability and how the new version of the generated code is fundamentally different from what we have today. I'm looking for answers from our security team. Without further information, I agree that we don't have to patch v0.21.0.

@roidelapluie
Copy link
Member

roidelapluie commented Feb 9, 2021

Sorry for the screenshot, that was the most practical I could find

skippy

As you can see, some of the code would be generated without checking that index+skippy < 0.
I guess the change below was moving both checks skippy < 0 and index+skippy < 0 on the same line to avoid copy/paste omissions in the future.

@simonpasquier
Copy link
Member Author

@roidelapluie 🙇 it was the missing piece to me, thanks a lot!

Fix for CVE-2021-3121

Signed-off-by: Simon Pasquier <spasquie@redhat.com>
@simonpasquier simonpasquier merged commit 23a7f89 into prometheus:master Feb 9, 2021
@simonpasquier simonpasquier deleted the bump-gogo-dep branch February 9, 2021 15:49
@beorn7
Copy link
Member

beorn7 commented Feb 9, 2021

Just for the record: The generated code that is actually used in Alertmanager is not affected by CVE-2021-3121. Therefore, we don't need to cut a bugfix release for 0.21.

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.

3 participants