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-2025-22868 #71490

Closed
thatnealpatel opened this issue Jan 30, 2025 · 2 comments
Closed

security: fix CVE-2025-22868 #71490

thatnealpatel opened this issue Jan 30, 2025 · 2 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. Security vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Milestone

Comments

@thatnealpatel
Copy link
Member

thatnealpatel commented Jan 30, 2025

This is a PRIVATE issue for CVE-2025-22868, tracked in http://b/392867809 and fixed by https://go-internal-review.git.corp.google.com/c/oauth2/+/1920.

/cc @golang/security and @golang/release

@thatnealpatel thatnealpatel added this to the Go1.25 milestone Jan 30, 2025
@gabyhelp gabyhelp added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Jan 30, 2025
@cagedmantis cagedmantis added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 31, 2025
@thatnealpatel thatnealpatel self-assigned this Feb 4, 2025
@rolandshoemaker
Copy link
Member

Fixed by https://go.dev/cl/652155.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/655455 mentions this issue: jws: improve fix for CVE-2025-22868

gopherbot pushed a commit to golang/oauth2 that referenced this issue Mar 13, 2025
The fix for CVE-2025-22868 relies on strings.Count, which isn't ideal
because it precludes failing fast when the token contains an unexpected
number of periods. Moreover, Verify still allocates more than necessary.

Eschew strings.Count in favor of strings.Cut. Some benchmark results:

goos: darwin
goarch: amd64
pkg: golang.org/x/oauth2/jws
cpu: Intel(R) Core(TM) i7-6700HQ CPU @ 2.60GHz
                              │      old       │                 new                 │
                              │     sec/op     │   sec/op     vs base                │
Verify/full_of_periods-8        24862.50n ± 1%   57.87n ± 0%  -99.77% (p=0.000 n=20)
Verify/two_trailing_periods-8      3.485m ± 1%   3.445m ± 1%   -1.13% (p=0.003 n=20)
geomean                            294.3µ        14.12µ       -95.20%

                              │     old      │                  new                   │
                              │     B/op     │     B/op      vs base                  │
Verify/full_of_periods-8          16.00 ± 0%     16.00 ± 0%        ~ (p=1.000 n=20) ¹
Verify/two_trailing_periods-8   2.001Mi ± 0%   1.001Mi ± 0%  -49.98% (p=0.000 n=20)
geomean                         5.658Ki        4.002Ki       -29.27%
¹ all samples are equal

                              │     old     │                 new                  │
                              │  allocs/op  │ allocs/op   vs base                  │
Verify/full_of_periods-8         1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=20) ¹
Verify/two_trailing_periods-8   12.000 ± 0%   9.000 ± 0%  -25.00% (p=0.000 n=20)
geomean                          3.464        3.000       -13.40%
¹ all samples are equal

Also, remove all remaining calls to strings.Split.

Updates golang/go#71490

Change-Id: Icac3c7a81562161ab6533d892ba19247d6d5b943
GitHub-Last-Rev: 3a82900
GitHub-Pull-Request: #774
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/655455
Commit-Queue: Neal Patel <nealpatel@google.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Auto-Submit: Neal Patel <nealpatel@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. Security vulncheck or vulndb Issues for the x/vuln or x/vulndb repo
Projects
None yet
Development

No branches or pull requests

6 participants