[LOW][Security] Fix biased random number generation

[pnappa]

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.

[pnappa]

@perry-mitchell , this is the fix for the issue I'd emailed you about a couple months ago!

Also tagging @alizain , as I'm not sure who's in charge of this repo anymore.

I'd recommend filing a CVE for this issue, it's definitely low severity, but worth getting people to upgrade to avoid any incidental problems with lower entropy.

[perry-mitchell]

Thanks @pnappa! Apologies for not helping it move forward faster. I'll get this patched immediately.