First, this is a great article showing the bias in Cryptocat's very awkward PRNG code.
However, the off-by-one bias was actually the least of the problems with Cryptocat's random numbers...
From reading Steve's write-up, the problem was that their keys were ridiculous undersized, because they called their own function wrong:
May 7, 2012 (switched from DH to ECC):
myPrivateKey = Cryptocat.randomString(32, 0, 0, 1);
April 19, 2013:
rand = Cryptocat.randomString(32, 0, 0, 1);
myPrivateKey = BigInt.str2bigInt(rand, 10);
June 3, 2013:
rand = Cryptocat.randomString(64, 0, 0, 1, 0);
myPrivateKey = BigInt.str2bigInt(rand, 16);
> The bug that lasted 347 days was the confusion between a string and an array of integers. This made the ECC private keys ridiculously small because they passed a string of decimal digits into a function expecting an array of 17, 15 bit integers. Each character was considered an element in the array. So each of those "15 bit integers" were only the values 0 to 9 (3.32 bits). Also the least significant 3 bits are zeroed giving you a key space of 2*10^16 (2^54.15). --
http://tobtu.com/decryptocat.phpSee for yourself here: https://github.com/cryptocat/cryptocat/commit/a17bb1599463e0...
Even now looking at this code, for me it just doesn't pass the 'WTFs per minute' test... It still looks fucking wrong to me... Cryptocat.randomString(64, 0, 0, 1, 0) returns a 64 character string of 0-9, why are they calling str2bigInt with a hex radix? Did they add the last '0' in the wrong place? Or maybe I'm looking at the wrong check in...
EDIT: Ok, it WAS still wrong as of June 4th... It wasn't until July 4th until it finally became:
var rand = Cryptocat.randomString(64, 0, 0, 0, 1)
See:
https://github.com/cryptocat/cryptocat/commit/8fb7f4b8e59c76...EDIT EDIT: So just to be clear... v2.1.11 which was supposed to fix this problem has this 'incorrect radix' bug in it? Please tell me I'm wrong, or do we need another CVE and another release?