-
Notifications
You must be signed in to change notification settings - Fork 188
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
fix: upgrade github.com/gogo/protobuf to v1.3.2 to fix CVE-2021-3121 for release-3.1 #346
fix: upgrade github.com/gogo/protobuf to v1.3.2 to fix CVE-2021-3121 for release-3.1 #346
Conversation
Welcome @mainred! |
Hi @mainred. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: mainred The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@davidz627 @saad-ali could you please take a look? |
go.mod
Outdated
@@ -89,3 +89,7 @@ replace k8s.io/controller-manager => k8s.io/controller-manager v0.20.0 | |||
replace k8s.io/csi-translation-lib => k8s.io/csi-translation-lib v0.20.0 | |||
|
|||
replace k8s.io/mount-utils => k8s.io/mount-utils v0.20.0 | |||
|
|||
replace github.com/gogo/protobuf => github.com/gogo/protobuf v1.3.2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not how you update dependencies. Let's also use this opportunity to update all dependencies to the next minor version, not just those that happen to have known issues.
Please use go get -t -u ./... && go mod tidy && go mod vendor
and submit that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out, let me re-organize other related PRs as well.
I added in the required version into the required field, then go get -t -u ./... && go mod tidy && go mod vendor
to upgrade the mod and vendor.
go.mod
Outdated
) | ||
|
||
replace github.com/googleapis/gnostic v0.5.3 => github.com/googleapis/gnostic v0.5.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
manually added this replacement to fix the namespace change from googleapis to google, google/gnostic#262
I'd like to point out that there is no CVE in the external-attacher. CVE-2021-3121 affects only code generated by protobuf v1.3.1. The external-attacher does not generate such code. There may be code in In addition, external-attacher v3.1.0 just went out of 1 year of support. |
Agree, external-attacher generates no CVE, so we change only the vendored code in external-attacher, not the code outside of vendor.
Thanks for pointing this out. |
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: