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
security: fix CVE-2022-41724 [1.20 backport] #58359
Labels
Milestone
Comments
gopherbot
added
the
CherryPickCandidate
Used during the release process for point releases
label
Feb 6, 2023
heschi
added
the
CherryPickApproved
Used during the release process for point releases
label
Feb 8, 2023
gopherbot
removed
the
CherryPickCandidate
Used during the release process for point releases
label
Feb 8, 2023
Change https://go.dev/cl/468121 mentions this issue: |
gopherbot
pushed a commit
that referenced
this issue
Feb 14, 2023
Message marshalling makes use of BytesOrPanic a lot, under the assumption that it will never panic. This assumption was incorrect, and specifically crafted handshakes could trigger panics. Rather than just surgically replacing the usages of BytesOrPanic in paths that could panic, replace all usages of it with proper error returns in case there are other ways of triggering panics which we didn't find. In one specific case, the tree routed by expandLabel, we replace the usage of BytesOrPanic, but retain a panic. This function already explicitly panicked elsewhere, and returning an error from it becomes rather painful because it requires changing a large number of APIs. The marshalling is unlikely to ever panic, as the inputs are all either fixed length, or already limited to the sizes required. If it were to panic, it'd likely only be during development. A close inspection shows no paths for a user to cause a panic currently. This patches ends up being rather large, since it requires routing errors back through functions which previously had no error returns. Where possible I've tried to use helpers that reduce the verbosity of frequently repeated stanzas, and to make the diffs as minimal as possible. Thanks to Marten Seemann for reporting this issue. Updates #58001 Fixes #58359 Fixes CVE-2022-41724 Change-Id: Ieb55867ef0a3e1e867b33f09421932510cb58851 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1679436 Reviewed-by: Julie Qiu <julieqiu@google.com> TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com> (cherry picked from commit 1d4e6ca9454f6cf81d30c5361146fb5988f1b5f6) Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728205 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/468121 Reviewed-by: Than McIntosh <thanm@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> TryBot-Bypass: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Pratt <mpratt@google.com>
Closed by merging 5286ac4 to release-branch.go1.20. |
romaindoumenc
pushed a commit
to TroutSoftware/go
that referenced
this issue
Mar 3, 2023
Message marshalling makes use of BytesOrPanic a lot, under the assumption that it will never panic. This assumption was incorrect, and specifically crafted handshakes could trigger panics. Rather than just surgically replacing the usages of BytesOrPanic in paths that could panic, replace all usages of it with proper error returns in case there are other ways of triggering panics which we didn't find. In one specific case, the tree routed by expandLabel, we replace the usage of BytesOrPanic, but retain a panic. This function already explicitly panicked elsewhere, and returning an error from it becomes rather painful because it requires changing a large number of APIs. The marshalling is unlikely to ever panic, as the inputs are all either fixed length, or already limited to the sizes required. If it were to panic, it'd likely only be during development. A close inspection shows no paths for a user to cause a panic currently. This patches ends up being rather large, since it requires routing errors back through functions which previously had no error returns. Where possible I've tried to use helpers that reduce the verbosity of frequently repeated stanzas, and to make the diffs as minimal as possible. Thanks to Marten Seemann for reporting this issue. Updates golang#58001 Fixes golang#58359 Fixes CVE-2022-41724 Change-Id: Ieb55867ef0a3e1e867b33f09421932510cb58851 Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1679436 Reviewed-by: Julie Qiu <julieqiu@google.com> TryBot-Result: Security TryBots <security-trybots@go-security-trybots.iam.gserviceaccount.com> Run-TryBot: Roland Shoemaker <bracewell@google.com> Reviewed-by: Damien Neil <dneil@google.com> (cherry picked from commit 1d4e6ca9454f6cf81d30c5361146fb5988f1b5f6) Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728205 Reviewed-by: Tatiana Bradley <tatianabradley@google.com> Reviewed-on: https://go-review.googlesource.com/c/go/+/468121 Reviewed-by: Than McIntosh <thanm@google.com> Auto-Submit: Michael Pratt <mpratt@google.com> TryBot-Bypass: Michael Pratt <mpratt@google.com> Run-TryBot: Michael Pratt <mpratt@google.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
@rolandshoemaker requested issue #58001 to be considered for backport to the next 1.20 minor release.
The text was updated successfully, but these errors were encountered: