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.

Sign up for GitHub

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

Jump to bottom

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
@thatnealpatel
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
Unreleased

Comments

@thatnealpatel
Copy link
Member

thatnealpatel commented Jan 30, 2025 •
edited by cagedmantis
Loading

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.

@dmitshur dmitshur modified the milestones: Go1.25, Unreleased Feb 25, 2025
@drakkan drakkan mentioned this issue Feb 25, 2025
Merged
@dims dims mentioned this issue Feb 27, 2025
Merged
@sxd sxd mentioned this issue Feb 27, 2025
Closed
4 tasks
@jub0bs jub0bs mentioned this issue Feb 28, 2025
Open
@janisz janisz mentioned this issue Mar 3, 2025
Merged
5 tasks
@ardaguclu ardaguclu mentioned this issue Mar 3, 2025
Merged
@jub0bs jub0bs mentioned this issue Mar 6, 2025
Closed
@gopherbot
Copy link
Contributor

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

This was referenced Mar 9, 2025
Closed
Open
@pavolloffay pavolloffay mentioned this issue Mar 12, 2025
Merged
gopherbot pushed a commit to golang/oauth2 that referenced this issue Mar 13, 2025
@gopherbot
jws: improve fix for CVE-2025-22868
ce56909 
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
Assignees

@thatnealpatel thatnealpatel

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
Milestone
Unreleased
Development

No branches or pull requests
6 participants
@cagedmantis @dmitshur @rolandshoemaker @gopherbot @thatnealpatel @gabyhelp