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

crypto/rand: Read hangs when passed buffer larger than 1<<32 - 1 #52561

Closed
rolandshoemaker opened this issue Apr 26, 2022 · 5 comments
Closed
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows Security
Milestone

Comments

@rolandshoemaker
Copy link
Member

rolandshoemaker commented Apr 26, 2022

Passing a buffer larger than 1<<32 - 1 to crypto/rand.Read hangs on windows due to an infinite loop because of how batching works with RtlGenRandom. Since RtlGenRandom only supports reading at most 1<<32 - 1 bytes at a time, rngReader truncates the requested number of bytes to uint32(len(b)) (or len(b) % 1 << 32). After the first call, which will return len(b) % 1 << 32 bytes, the truncation will always result in 0, causing the infinite loop.

Since this requires such a large buffer, this has minimal impact, since it's incredibly unlikely anyone actually wants this much randomness (and there are no paths from the remotely reachable libraries where this can be realistically triggered.)

This is CVE-2022-30634.

@rolandshoemaker rolandshoemaker added OS-Windows NeedsFix The path to resolution is known, but the work has not been done. labels Apr 26, 2022
@gopherbot
Copy link

Change https://go.dev/cl/402257 mentions this issue: crypto/rand: properly handle large Read on windows

@rolandshoemaker
Copy link
Member Author

@gopherbot please open backport issues, this is a minor security issue.

@gopherbot
Copy link

Backport issue(s) opened: #52932 (for 1.17), #52933 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot
Copy link

Change https://go.dev/cl/406635 mentions this issue: [release-branch.go1.17] crypto/rand: properly handle large Read on windows

@gopherbot
Copy link

Change https://go.dev/cl/406634 mentions this issue: [release-branch.go1.18 crypto/rand: properly handle large Read on windows

gopherbot pushed a commit that referenced this issue May 25, 2022
…handle large Read on windows

Use the batched reader to chunk large Read calls on windows to a max of
1 << 31 - 1 bytes. This prevents an infinite loop when trying to read
more than 1 << 32 -1 bytes, due to how RtlGenRandom works.

This change moves the batched function from rand_unix.go to rand.go,
since it is now needed for both windows and unix implementations.

Updates #52561
Fixes #52933
Fixes CVE-2022-30634

Change-Id: Id98fc4b1427e5cb2132762a445b2aed646a37473
Reviewed-on: https://go-review.googlesource.com/c/go/+/402257
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit bb1f441)
Reviewed-on: https://go-review.googlesource.com/c/go/+/406634
Reviewed-by: Damien Neil <dneil@google.com>
gopherbot pushed a commit that referenced this issue May 25, 2022
…ndows

Use the batched reader to chunk large Read calls on windows to a max of
1 << 31 - 1 bytes. This prevents an infinite loop when trying to read
more than 1 << 32 -1 bytes, due to how RtlGenRandom works.

This change moves the batched function from rand_unix.go to rand.go,
since it is now needed for both windows and unix implementations.

Updates #52561
Fixes #52932
Fixes CVE-2022-30634

Change-Id: Id98fc4b1427e5cb2132762a445b2aed646a37473
Reviewed-on: https://go-review.googlesource.com/c/go/+/402257
Run-TryBot: Roland Shoemaker <roland@golang.org>
Reviewed-by: Filippo Valsorda <filippo@golang.org>
Reviewed-by: Filippo Valsorda <valsorda@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit bb1f441)
Reviewed-on: https://go-review.googlesource.com/c/go/+/406635
Reviewed-by: Damien Neil <dneil@google.com>
@dmitshur dmitshur added this to the Go1.19 milestone May 31, 2022
@golang golang locked and limited conversation to collaborators May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows Security
Projects
None yet
Development

No branches or pull requests

3 participants