diff --git a/lib/internal/crypto/random.js b/lib/internal/crypto/random.js index c53494aed848..0dc03e236a00 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 000000000000..027793aa1fbb --- /dev/null +++ b/test/parallel/test-crypto-randomint-cache.js @@ -0,0 +1,43 @@ +// 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 { internalBinding } = require('internal/test/binding'); +const { RandomBytesJob } = internalBinding('crypto'); + +// 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 = []; + +// The refill job calls ondone on the main thread after the threadpool has +// written its buffer. Make the synchronous calls at that point, before the +// refill callback in randomInt() runs. +const { run } = RandomBytesJob.prototype; +RandomBytesJob.prototype.run = common.mustCall(function() { + RandomBytesJob.prototype.run = run; + const { ondone } = this; + this.ondone = common.mustCall((...args) => { + for (let i = 0; i < 3; i++) + values.push(randomInt(max)); + ondone(...args); + }); + run.call(this); +}); + +// 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}`); +}));