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

security: fix CVE-2022-41724 [1.19 backport] #58358

Closed
gopherbot opened this issue Feb 6, 2023 · 2 comments
Closed

security: fix CVE-2022-41724 [1.19 backport] #58358

gopherbot opened this issue Feb 6, 2023 · 2 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Security
Milestone

Comments

@gopherbot
Copy link

@rolandshoemaker requested issue #58001 to be considered for backport to the next 1.19 minor release.

@gopherbot please open backport issues.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 6, 2023
@gopherbot gopherbot added this to the Go1.19.6 milestone Feb 6, 2023
@heschi heschi added the CherryPickApproved Used during the release process for point releases label Feb 8, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 8, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/468117 mentions this issue: [release-branch.go1.19] crypto/tls: replace all usages of BytesOrPanic

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 #58358
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 0f3a44ad7b41cc89efdfad25278953e17d9c1e04)
Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1728204
Reviewed-by: Tatiana Bradley <tatianabradley@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/468117
Auto-Submit: Michael Pratt <mpratt@google.com>
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Than McIntosh <thanm@google.com>
@gopherbot
Copy link
Author

Closed by merging 00b256e to release-branch.go1.19.

@golang golang locked and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge Security
Projects
None yet
Development

No branches or pull requests

3 participants