Repository navigation
Conversation
The Linux randomBytes() loop computed the resume pointer p for each iteration but then passed the original buffer start (pbytes) to the getrandom(2) syscall. After a partial read, every retry overwrote the beginning of the buffer while res kept accumulating, so the tail of the buffer was never written and randomBytes() still reported full success. Fixes cheatfate#103.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #103.
The bug
In the Linux implementation of
randomBytes()the retry loop for partialgetrandom(2)reads computed the resume pointerp, but then passed the original buffer startpbytesto the syscall:After a partial read of
kbytes, every retry restarted at the buffer start whilereskept accumulating, so the region[k, nbytes)was never written and the function still returned full success. For a CSPRNG API that means keys/nonces/salts can silently contain uninitialized memory — exactly as described in #103.The fix
One line: pass
pinstead ofpbytes, matching how the other implementations in this file (urandomRead, the OpenBSD path) already handle the resume pointer.Verification
I could not force a partial read deterministically from userspace (this kernel fills large requests in a single call), so I verified the resume path with a small
LD_PRELOADharness that interceptslibc syscall()and makes the firstgetrandomcall withlen > 8return a partial read of 8 bytes, passing later calls through unchanged. A checker program fills a 256-byte buffer with the0xABmarker, callsrandomBytes(), and reports how many stale marker bytes remain after the first 8 positions:So with the old code the tail of the buffer (248 of 256 bytes) kept its pre-call contents while the function reported success; with the fix the whole buffer is filled.
tests/testsysrand.nimpasses in all four compile modes used bynimble test(default,-d:danger,--threads:on,--mm:orc --threads:on) on Nim 2.2.4 / Linux amd64.I did not add a regression test because a partial
getrandom(2)read can't be triggered deterministically from a normal unit test (it depends on signal timing / kernel limits), and mocking would require refactoring the direct syscall import. Happy to add one if you'd prefer a refactor that injects the syscall — happy to follow up in a separate PR.