crypto: avoid reusing randomInt() cache bytes

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

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));