From 59e0b8478e03af6838bfc84be565c46876261e3f Mon Sep 17 00:00:00 2001
From: Jiawen Geng <technicalcute@gmail.com>
Date: Thu, 8 Oct 2026 15:32:22 +0800
Subject: [PATCH] crypto: avoid reusing randomInt() cache bytes

When an asynchronous randomInt() call finds the cache empty, it queues
itself and starts randomFill() on the shared cache buffer. A synchronous
call made before that job completes refills the same buffer with
randomFillSync() and returns values from offset 0. The completion
callback then reset the offset to 0, so the queued call and later calls
returned the bytes that synchronous calls had already used. The
threadpool job and randomFillSync() could also write the buffer at the
same time.

Fill a separate buffer asynchronously and copy it into the cache once
the job is done.

Fixes: https://github.com/nodejs/node/issues/66595
Refs: https://github.com/nodejs/node/pull/35110
Signed-off-by: Jiawen Geng <technicalcute@gmail.com>
Assisted-by: claude:opus-5.5
---
 lib/internal/crypto/random.js                | 13 ++++++--
 test/parallel/test-crypto-randomint-cache.js | 33 ++++++++++++++++++++
 2 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-crypto-randomint-cache.js

diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js
index c53494aed84..0dc03e236a0 100644
--- a/lib/internal/crypto/random.js
+++ b/lib/internal/crypto/random.js
@@ -25,6 +25,7 @@ const {
   TypedArrayPrototypeGetByteLength,
   TypedArrayPrototypeGetLength,
   TypedArrayPrototypeGetSymbolToStringTag,
+  TypedArrayPrototypeSet,
   Uint8Array,
 } = primordials;
 
@@ -233,6 +234,8 @@ const RAND_MAX = 0xFFFF_FFFF_FFFF;
 // divisible by 6 because each attempt to obtain a random int uses 6 bytes.
 const randomCache = new FastBuffer(6 * 1024);
 let randomCacheOffset = randomCache.length;
+// Asynchronous refills write into this buffer, never into randomCache.
+let asyncRandomCache;
 let asyncCacheFillInProgress = false;
 const asyncCachePendingTasks = [];
 
@@ -313,13 +316,19 @@ function asyncRefillRandomIntCache() {
     return;
 
   asyncCacheFillInProgress = true;
-  randomFill(randomCache, (err) => {
+  // Synchronous calls may refill and read randomCache while this job runs.
+  // Filling a separate buffer and copying it in afterwards ensures that
+  // resetting the offset never hands out bytes they have already used.
+  asyncRandomCache ??= new FastBuffer(randomCache.length);
+  randomFill(asyncRandomCache, (err) => {
     asyncCacheFillInProgress = false;
 
     const tasks = asyncCachePendingTasks;
     const errorReceiver = err && ArrayPrototypeShift(tasks);
-    if (!err)
+    if (!err) {
+      TypedArrayPrototypeSet(randomCache, asyncRandomCache);
       randomCacheOffset = 0;
+    }
 
     // Restart all pending tasks. If an error occurred, we only notify a single
     // callback (errorReceiver) about it. This way, every async call to
diff --git a/test/parallel/test-crypto-randomint-cache.js b/test/parallel/test-crypto-randomint-cache.js
new file mode 100644
index 00000000000..fb9bed86650
--- /dev/null
+++ b/test/parallel/test-crypto-randomint-cache.js
@@ -0,0 +1,33 @@
+// Flags: --expose-internals
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto)
+  common.skip('missing crypto');
+
+// Synchronous randomInt() calls made while an asynchronous cache refill is
+// pending must not cause the same random bytes to be returned twice.
+
+const assert = require('assert');
+const { randomInt } = require('crypto');
+const { sleep } = require('internal/util');
+
+// With this range, every 6-byte draw except 0xffffffffffff is returned
+// unchanged, so a repeated value means repeated cache bytes.
+const max = 2 ** 48 - 1;
+const values = [];
+
+// This is the first randomInt() call in the process, so the cache is empty.
+// The call is queued and an asynchronous refill starts.
+randomInt(max, common.mustSucceed((n) => {
+  values.push(n);
+  for (let i = 0; i < 3; i++)
+    values.push(randomInt(max));
+  assert.strictEqual(new Set(values).size, values.length,
+                     `duplicate values: ${values}`);
+}));
+
+// Let the refill job finish before the synchronous calls refill the cache.
+sleep(100);
+
+for (let i = 0; i < 3; i++)
+  values.push(randomInt(max));