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

rand: Fix OsRng::fill_bytes on Windows #31876

Merged
merged 1 commit into from
Feb 27, 2016
Merged

Conversation

ollie27
Copy link
Member

@ollie27 ollie27 commented Feb 25, 2016

CryptGenRandom takes a DWORD (u32) for the length so it only supports
writing u32::MAX bytes at a time.

Casting the length from a usize caused truncation meaning the whole
buffer was not always filled.

cc #31841

This is the same as rust-random/rand#99. I think it's a good idea to keep the implementations in sync.

r? @alexcrichton

…indows

CryptGenRandom takes a DWORD (u32) for the length so it only supports
writing u32::MAX bytes at a time.

Casting the length from a usize caused truncation meaning the whole
buffer was not always filled.
@brson brson added the relnotes Marks issues that should be documented in the release notes of the next release. label Feb 25, 2016
@brson
Copy link
Contributor

brson commented Feb 25, 2016

@bors r+

Look like a security bug cc @rust-lang/core

@bors
Copy link
Contributor

bors commented Feb 25, 2016

📌 Commit ac3cc33 has been approved by brson

@emberian
Copy link
Member

it is a security bug, but note that what really matters is the external crate, not this one. rust-random/rand#99

@bors
Copy link
Contributor

bors commented Feb 26, 2016

⌛ Testing commit ac3cc33 with merge 4236392...

@bors
Copy link
Contributor

bors commented Feb 26, 2016

💔 Test failed - auto-mac-32-opt

@alexcrichton
Copy link
Member

@bors: retry

On Fri, Feb 26, 2016 at 11:22 AM, bors notifications@github.com wrote:

[image: 💔] Test failed - auto-mac-32-opt
http://buildbot.rust-lang.org/builders/auto-mac-32-opt/builds/8236


Reply to this email directly or view it on GitHub
#31876 (comment).

bors added a commit that referenced this pull request Feb 26, 2016
CryptGenRandom takes a DWORD (u32) for the length so it only supports
writing u32::MAX bytes at a time.

Casting the length from a usize caused truncation meaning the whole
buffer was not always filled.

cc #31841

This is the same as rust-random/rand#99. I think it's a good idea to keep the implementations in sync.

r? @alexcrichton
@bors
Copy link
Contributor

bors commented Feb 26, 2016

⌛ Testing commit ac3cc33 with merge 98a8a71...

@bors bors merged commit ac3cc33 into rust-lang:master Feb 27, 2016
@ollie27 ollie27 deleted the win_fill_bytes branch May 2, 2016 18:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants