From 58bd2b63f0e8593d41f3408b1ecd724c464ec6ad Mon Sep 17 00:00:00 2001
From: Jordan Milne <jmilne@uber.com>
Date: Fri, 14 Oct 2016 17:38:33 -0700
Subject: [PATCH] Generate higher-entropy UIDs for placeholders

---
 index.js               | 38 ++++++++++++++++++++++++++++++++------
 package.json           |  3 +++
 test/unit/serialize.js | 30 ++++++++++++------------------
 3 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/index.js b/index.js
index b1484d8..98aa790 100644
--- a/index.js
+++ b/index.js
@@ -6,11 +6,13 @@ See the accompanying LICENSE file for terms.
 
 'use strict';
 
-var isRegExp = require('util').isRegExp;
+var isRegExp    = require('util').isRegExp;
+var randomBytes = require('randombytes');
 
 // Generate an internal UID to make the regexp pattern harder to guess.
-var UID                 = Math.floor(Math.random() * 0x10000000000).toString(16);
-var PLACE_HOLDER_REGEXP = new RegExp('(\\\\)?"@__(F|R)-' + UID + '-(\\d+)__@"', 'g');
+var UID_LENGTH          = 16;
+var UID                 = generateUID();
+var PLACE_HOLDER_REGEXP = buildPlaceHolderRegExp(UID);
 
 var IS_NATIVE_CODE_REGEXP = /\{\s*\[native code\]\s*\}/g;
 var UNSAFE_CHARS_REGEXP   = /[<>\/\u2028\u2029]/g;
@@ -29,6 +31,19 @@ function escapeUnsafeChars(unsafeChar) {
     return ESCAPED_CHARS[unsafeChar];
 }
 
+function generateUID() {
+    var bytes = randomBytes(UID_LENGTH);
+    var result = '';
+    for(var i=0; i<UID_LENGTH; ++i) {
+        result += bytes[i].toString(16);
+    }
+    return result;
+}
+
+function buildPlaceHolderRegExp(uid) {
+    return new RegExp('(\\\\)?"@__(F|R)-' + uid + '-(\\d+)__@"', 'g');
+}
+
 module.exports = function serialize(obj, options) {
     options || (options = {});
 
@@ -37,6 +52,7 @@ module.exports = function serialize(obj, options) {
         options = {space: options};
     }
 
+    var uid = options.uid || UID;
     var functions = [];
     var regexps   = [];
 
@@ -51,14 +67,14 @@ module.exports = function serialize(obj, options) {
 
         if (type === 'object') {
             if (isRegExp(value)) {
-                return '@__R-' + UID + '-' + (regexps.push(value) - 1) + '__@';
+                return '@__R-' + uid + '-' + (regexps.push(value) - 1) + '__@';
             }
 
             return value;
         }
 
         if (type === 'function') {
-            return '@__F-' + UID + '-' + (functions.push(value) - 1) + '__@';
+            return '@__F-' + uid + '-' + (functions.push(value) - 1) + '__@';
         }
 
         return value;
@@ -89,10 +105,20 @@ module.exports = function serialize(obj, options) {
         return str;
     }
 
+    var placeHolderRegExp;
+    if (options.uid) {
+        placeHolderRegExp = buildPlaceHolderRegExp(uid);
+    } else {
+        placeHolderRegExp = PLACE_HOLDER_REGEXP;
+    }
+
     // Replaces all occurrences of function and regexp placeholders in the JSON
     // string with their string representations. If the original value can not
     // be found, then `undefined` is used.
-    return str.replace(PLACE_HOLDER_REGEXP, function (match, backSlash, type, valueIndex) {
+    return str.replace(placeHolderRegExp, function (match, backSlash, type, valueIndex) {
+        // The placeholder may not be preceded by a backslash. This is to prevent
+        // replacing things like `"a\"@__R-<UID>-0__@"` and thus outputting
+        // invalid JS.
         if (backSlash) {
             return match;
         }
diff --git a/package.json b/package.json
index f8950e1..ea4509e 100644
--- a/package.json
+++ b/package.json
@@ -30,5 +30,8 @@
     "istanbul": "^0.3.2",
     "mocha": "^1.21.4",
     "xunit-file": "0.0.5"
+  },
+  "dependencies": {
+    "randombytes": "^2.0.3"
   }
 }
diff --git a/test/unit/serialize.js b/test/unit/serialize.js
index 89b76d3..68a14cd 100644
--- a/test/unit/serialize.js
+++ b/test/unit/serialize.js
@@ -1,15 +1,9 @@
 /* global describe, it, beforeEach */
 'use strict';
 
-// Temporarily replace `Math.random` so `UID` will be deterministic
-var oldRandom = Math.random;
-Math.random = function(){return 0.5};
-
 var serialize = require('../../'),
     expect    = require('chai').expect;
 
-Math.random = oldRandom;
-
 describe('serialize( obj )', function () {
     it('should be a function', function () {
         expect(serialize).to.be.a('function');
@@ -117,6 +111,18 @@ describe('serialize( obj )', function () {
         });
     });
 
+    describe('placeholders', function() {
+        it('should not be replaced within string literals', function () {
+            // Since we made the UID deterministic this should always be the placeholder
+            var fakePlaceholder = '"@__R-foo-0__@';
+            var serialized = serialize({bar: /1/i, foo: fakePlaceholder}, {uid: 'foo'});
+            var obj = eval('(' + serialized + ')');
+            expect(obj).to.be.a('Object');
+            expect(obj.foo).to.be.a('String');
+            expect(obj.foo).to.equal(fakePlaceholder);
+        });
+    });
+
     describe('regexps', function () {
         it('should serialize constructed regexps', function () {
             var re = new RegExp('asdf');
@@ -166,18 +172,6 @@ describe('serialize( obj )', function () {
             expect(re).to.be.a('RegExp');
             expect(re.source).to.equal('\\..*');
         });
-
-        it('should ignore placeholders with leading backslashes', function(){
-            // Since we made the UID deterministic this should always be the placeholder
-            var placeholder = '@__R-8000000000-0__@';
-            var obj = eval('(' + serialize({
-                "bar": /1/i,
-                "foo": '"' + placeholder
-            }) + ')');
-            expect(obj).to.be.a('Object');
-            expect(obj.foo).to.be.a('String');
-            expect(obj.foo).to.equal('"' + placeholder);
-        });
     });
 
     describe('XSS', function () {