Skip to content

fix(sysrand): resume getrandom at correct offset after short reads - #104

Open
cryo2010 wants to merge 3 commits into
cheatfate:masterfrom
cryo2010:fix/103-sysrand-getrandom-partial-read
Open

fix(sysrand): resume getrandom at correct offset after short reads#104
cryo2010 wants to merge 3 commits into
cheatfate:masterfrom
cryo2010:fix/103-sysrand-getrandom-partial-read

Conversation

@cryo2010

Copy link
Copy Markdown

Summary

Fixes #103.

On Linux, randomBytes fills the caller's buffer by looping over getrandom(2), which may return fewer bytes than requested for large buffers. The loop computed the resume pointer p but passed the buffer start pbytes to the syscall on every iteration:

p = cast[pointer](cast[uint](pbytes) + uint(res))
let bytesRead = syscall(SYS_getrandom, pbytes, nbytes - res, 0)  # should be p

After a partial read, subsequent reads overwrote the beginning of the buffer and left the tail untouched, while res still accumulated to nbytes so the routine reported full success. Callers could receive predictable (uninitialized) bytes in the tail of keys, nonces, or salts.

Changes

  • refactor: extract the short-read offset-accumulation loop into a reusable fillRandomBytes helper that takes a reader callback. This is behavior-preserving and creates a testable seam (the raw getrandom loop was previously not unit-testable, since partial reads are nondeterministic).
  • test: add a regression test that drives fillRandomBytes with a mock reader emulating short reads (<= 8 bytes/call) and asserts the whole buffer is written.
  • fix: pass the resumed offset p to the reader so the buffer is filled from the correct position.

Testing

nim c -r --path:. tests/testsysrand.nim passes all cases, including the new "getrandom partial-read offset test". The test fails on the pre-fix code and passes after the one-line fix.

Extract the short-read offset-accumulation loop from the Linux getrandom
path into a reusable fillRandomBytes helper that takes a reader callback.
This is behavior-preserving and introduces a testable seam so the
partial-read handling can be covered by unit tests (issue cheatfate#103).

The test and the accompanying fix will follow in a later commit.
… offset

Drive fillRandomBytes with a mock reader that emulates short reads (at
most 8 bytes per call). The offset bug causes each call to overwrite the
front of the buffer, leaving the tail unwritten while still reporting
full success.

This test fails against the current code and will pass once the offset
fix lands (issue cheatfate#103).
The Linux getrandom fill loop passed the buffer start (pbytes) to the
reader on every iteration instead of the current offset (p). After a
partial read, subsequent reads overwrote the beginning of the buffer and
left the tail uninitialized, while the routine still reported full
success. Callers could receive predictable bytes in the tail of keys,
nonces, or salts.

Pass the resumed offset p to the reader so the whole buffer is filled.

Fixes cheatfate#103.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] randomBytes on Linux can return non-random, uninitialized memory

1 participant