From 6888a9a7fd5479b449609d270c0e2f0ab267ed6a Mon Sep 17 00:00:00 2001 From: Craig Younker Date: Fri, 21 Aug 2026 09:25:22 -0700 Subject: [PATCH 1/3] refactor(sysrand): extract fillRandomBytes helper to enable unit testing 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 #103). The test and the accompanying fix will follow in a later commit. --- nimcrypto/sysrand.nim | 48 ++++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/nimcrypto/sysrand.nim b/nimcrypto/sysrand.nim index a90e280..70d7518 100644 --- a/nimcrypto/sysrand.nim +++ b/nimcrypto/sysrand.nim @@ -26,6 +26,29 @@ {.push raises: [].} +type + RandomFillProc* = proc(pbytes: pointer, nbytes: int): int {.gcsafe, raises: [].} + ## Reader used by `fillRandomBytes`. It should write up to `nbytes` bytes at + ## `pbytes` and return the number of bytes produced (> 0), `0` to stop, or a + ## negative value to indicate a retryable condition (e.g. `EINTR`). + +proc fillRandomBytes*(pbytes: pointer, nbytes: int, + reader: RandomFillProc): int = + ## Fill `nbytes` of memory at `pbytes` by repeatedly invoking `reader`, + ## resuming at the correct offset after short reads. Returns the number of + ## bytes actually written. + var res = 0 + while res < nbytes: + let p = cast[pointer](cast[uint](pbytes) + uint(res)) + let bytesRead = reader(pbytes, nbytes - res) + if bytesRead > 0: + res += bytesRead + elif bytesRead == 0: + break + else: + discard + res + when defined(posix): import os, posix @@ -128,27 +151,24 @@ elif defined(linux): gSystemRng = newSystemRng() gSystemRng + proc getrandomReader(p: pointer, n: int): int {.gcsafe, raises: [].} = + let r = int(syscall(SYS_getrandom, p, n, 0)) + if r >= 0: + r + elif osLastError().int32 == EINTR: + -1 + else: + 0 + proc randomBytes*(pbytes: pointer, nbytes: int): int = - var p: pointer let srng = getSystemRng() if srng.getRandomPresent: - var res = 0 - while res < nbytes: - p = cast[pointer](cast[uint](pbytes) + uint(res)) - let bytesRead = syscall(SYS_getrandom, pbytes, nbytes - res, 0) - if bytesRead > 0: - res += bytesRead - elif bytesRead == 0: - break - else: - if osLastError().int32 != EINTR: - break - + var res = fillRandomBytes(pbytes, nbytes, getrandomReader) if res <= 0: res = urandomRead(pbytes, nbytes) elif res < nbytes: - p = cast[pointer](cast[uint](pbytes) + uint(res)) + let p = cast[pointer](cast[uint](pbytes) + uint(res)) let bytesRead = urandomRead(p, nbytes - res) if bytesRead != -1: res += bytesRead From a76fd1eceedb9a1faba27f98a8aad7e1da062227 Mon Sep 17 00:00:00 2001 From: Craig Younker Date: Fri, 21 Aug 2026 09:27:41 -0700 Subject: [PATCH 2/3] test(sysrand): add failing regression test for getrandom partial-read 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 #103). --- tests/testsysrand.nim | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/testsysrand.nim b/tests/testsysrand.nim index e7a0bde..cda8575 100644 --- a/tests/testsysrand.nim +++ b/tests/testsysrand.nim @@ -41,3 +41,31 @@ suite "OS random source Tests": result = true check test() == true + + test "getrandom partial-read offset test": + # A source such as getrandom(2) may return fewer bytes than requested for + # large buffers. `fillRandomBytes` must resume at the correct offset after + # each short read; otherwise later reads overwrite the beginning of the + # buffer and leave the tail untouched while still reporting full success. + const Total = 64 + var buffer: array[Total, byte] + + proc shortReader(pbytes: pointer, nbytes: int): int {.gcsafe, raises: [].} = + # Emulate a source that only ever produces up to 8 bytes per call, writing + # a non-zero marker so an untouched tail is detectable. + let chunk = min(nbytes, 8) + let arr = cast[ptr UncheckedArray[byte]](pbytes) + for i in 0 ..< chunk: + arr[i] = 0xAA'u8 + chunk + + let filled = fillRandomBytes(addr buffer[0], Total, shortReader) + + var allWritten = true + for b in buffer: + if b != 0xAA'u8: + allWritten = false + + check: + filled == Total + allWritten == true From 800ce11e266c2f86b54e1f84a7588a8f2ff02923 Mon Sep 17 00:00:00 2001 From: Craig Younker Date: Fri, 21 Aug 2026 09:29:21 -0700 Subject: [PATCH 3/3] fix(sysrand): resume getrandom at correct offset after short reads 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 #103. --- nimcrypto/sysrand.nim | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nimcrypto/sysrand.nim b/nimcrypto/sysrand.nim index 70d7518..43b4d07 100644 --- a/nimcrypto/sysrand.nim +++ b/nimcrypto/sysrand.nim @@ -40,7 +40,7 @@ proc fillRandomBytes*(pbytes: pointer, nbytes: int, var res = 0 while res < nbytes: let p = cast[pointer](cast[uint](pbytes) + uint(res)) - let bytesRead = reader(pbytes, nbytes - res) + let bytesRead = reader(p, nbytes - res) if bytesRead > 0: res += bytesRead elif bytesRead == 0: