Please don't redefine Math.random() to be non random

[kevinwbaker]

Hey wwwtyro,

I'm a developer of 3rd party JavaScript tools that my customers install on their websites. I ran across cryptico, while debugging why my tools broke on my customer's website. The customer had installed cryptico, and it had redefined the global function Math.random() to be non-random.

We all share the global namespace. Could you please reconsider this approach as it breaks the functionality of other scripts on the page that rely on Math.random() to be pseudo-random.

[LandonPowell]

I've got plenty of issues with the code quality of cryptico, but I don't fully understand what you're saying here.

Math.random() doesn't become any more or less random because of what he did. It's still equally pseudo random, but now he's just made it easy to define his own seed.

I'm very curious as to how his modifications to Math.random() broke your JS tool though. It's commonly accepted to use drop-in modifications to Math.random() to implement the ability to seed with it.

[drpeck]

I guess the point is that as soon as some code uses Math.seedrandom(), all other areas of functionality on a site then get predictable random numbers. This has broken MouseFlow on my clients website.

[trun]

@drpeck has it right - we also witnessed a similar side effect on a client's website stemming from poor use of cryptico. In a nutshell Math.seedrandom() was being called with a fixed seed which caused all subsequent calls to Math.random() to be deterministic. Is there any reason to assign these functions to the global namespace? This comment leads me to believe that use of the global namespace is unintentional.

[ramsestom]

If there is no reason to override Math.random globally, then there is no reason to call it Math.random either. (just call it Math.myrandom for example, or whatever, to avoid confusion). So it there is this global override, it might be that this modified version should be used from other global functions that use Math.random()... But anyway good practice would have been to rewritte all these modified functions and change their names, and certainly not to override them...

[DevBrent]

I just spent several hours tracking down why our array shuffling was deterministic on boot and am very surprised by the lack of outrage and offended by any justifications regarding this matter.

wwwtyro could have easily not monkey patched Math.random() by calling var myrng = new Math.seedrandom('hello.'); rather than just Math.seedrandom('hello.');

Whichever secret passphrase you use last dictates the random numbers you get everywhere in your app repeatably. Code sample:

const cryptico = require('cryptico');
const encryption_key = cryptico.generateRSAKey('uZq%ogoYtt^%', 1024);
const predicted_value = 0.006809886770680776;
const rand = Math.random();
console.log('I can\'t believe you\'ve done this.', rand === predicted_value ? 'Yes' : 'No', rand);

Solution is to randomly re-seed using the non-standard Math.seedrandom() after every use of generateRSAKey using this module. If you have cryptico installed you already have Math.seedrandom() available.
Edit: You can also do this since monkey patching is the season.

var cryptico = require('cryptico');
var origGenerateRSAKey = cryptico.generateRSAKey;
cryptico.generateRSAKey = function (phrase, bits) {  
  var key = origGenerateRSAKey(phrase, bits);
  Math.seedrandom();
  return key;
};