Skip to content

Commit f100a81

Browse files
committed
[LOW][Security] Fix biased random number generation
The random number generation logic results in a bias leading to some characters being more frequent than others, due to incorrect processing of random bytes. This can result in more easily guessable IDs, enabling some enumeration of resources. The crux of the vulnerability is that the RNG (as defined in `detectPRNG`), generates a float by generating a random byte and dividing by 255 (0xff). So, if the random byte is 0x00, it returns 0. If the byte is 0x01, it returns ~0.0039, and so on, until if the random byte is 0xff, it returns 1. Thus, the float returned could be [0, 1], rather than [0, 1) (as some comments expect). For reference, The whole list of possible outputs of the rng function returned by `detectPRNG` can be obtained by the following snippet: ``` const rngValues = Array.from(new Array(256), (_, idx) => (idx)/0xff); > [0, 0.0039, …, 1] ``` As a result, the `randomChar` function (as used by `encodeRandom`) will generate a bias, as it wraps around if the value is 1. All possible randomPosition values are thus (where 32 is from `ENCODING_LEN`): ``` const randomPositions = rngValues.map((v) => Math.floor(v * 32) % 32) > [0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, …, 31, 0] ``` If you take a look at the values of this array, you’ll notice that the distribution isn’t equal: ``` randomPositions.reduce((acc, v) => { if (!acc.has(v)) { acc.set(v, 0); } acc.set(v, acc.get(v)+1); return acc; }, new Map()); > { 0 => 9, 1 => 8, 2 => 8, …, 30 => 8, 31 => 7 } ``` Therefore, it’s more likely to generate a ‘0’ character, than a ‘Z’ character, and thus will lead to bias, thus a lower entropy than expected.
1 parent 09c28ab commit f100a81

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

source/ulid.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ export function detectPRNG(root?: any): PRNG {
5252
return () => {
5353
const buffer = new Uint8Array(1);
5454
globalCrypto.getRandomValues(buffer);
55-
return buffer[0] / 0xff;
55+
return buffer[0] / 256;
5656
};
5757
} else if (typeof globalCrypto?.randomBytes === "function") {
58-
return () => globalCrypto.randomBytes(1).readUInt8() / 0xff;
58+
return () => globalCrypto.randomBytes(1).readUInt8() / 256;
5959
} else if (crypto?.randomBytes) {
60-
return () => crypto.randomBytes(1).readUInt8() / 0xff;
60+
return () => crypto.randomBytes(1).readUInt8() / 256;
6161
}
6262
throw new ULIDError(ULIDErrorCode.PRNGDetectFailure, "Failed to find a reliable PRNG");
6363
}

0 commit comments

Comments
 (0)