fix: SAML assertion/response IDs generated with Math.random

lib/utils.js

@@ -1,3 +1,5 @@
+var crypto = require('crypto');
+
 exports.pemToCert = function(pem) {
   var cert = /-----BEGIN CERTIFICATE-----([^-]*)-----END CERTIFICATE-----/g.exec(pem.toString());
   if (cert && cert.length > 0) {
@@ -28,10 +30,16 @@ exports.reportError = function(err, callback){
 exports.uid = function(len) {
   var buf = []
     , chars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789'
-    , charlen = chars.length;
-
-  for (var i = 0; i < len; ++i) {
-    buf.push(chars[getRandomInt(0, charlen - 1)]);
+    , charlen = chars.length
+    , maxValid = charlen * Math.floor(256 / charlen); // reject values that would bias the modulo
+
+  while (buf.length < len) {
+    var bytes = crypto.randomBytes(len - buf.length);
+    for (var i = 0; i < bytes.length && buf.length < len; ++i) {
+      if (bytes[i] < maxValid) {
+        buf.push(chars[bytes[i] % charlen]);
+      }
+    }
   }
 
   return buf.join('');
@@ -45,16 +53,3 @@ exports.removeWhitespace = function(xml) {
                 .trim();
   return trimmed;
 };
-
-/**
- * Retrun a random int, used by `utils.uid()`
- *
- * @param {Number} min
- * @param {Number} max
- * @return {Number}
- * @api private
- */
-
-function getRandomInt(min, max) {
-  return Math.floor(Math.random() * (max - min + 1)) + min;
-};

test/utils.tests.js

@@ -8,4 +8,19 @@ describe("saml 1.1", function() {
 			assert.ok(!cert);
 		});
 	});
+
+	describe("uid", function() {
+		it("should not rely on Math.random to generate identifiers", function() {
+			var originalRandom = Math.random;
+			Math.random = function() { return 0; };
+			try {
+				var uid = utils.uid(32);
+				assert.equal(uid.length, 32);
+				assert.ok(/^[A-Za-z0-9]{32}$/.test(uid));
+				assert.notEqual(uid, new Array(33).join('A'));
+			} finally {
+				Math.random = originalRandom;
+			}
+		});
+	});
 });