Don't replace regex / function placeholders within string literals (#79)

Previously we weren't checking if the quote that started the placeholder was escaped or not, meaning an object like {"foo": /1"/, "bar": "a\"@__R-<UID>-0__@"} Would be serialized as {"foo": /1"/, "bar": "a\/1"/} meaning an attacker could escape out of `bar` if they controlled both `foo` and `bar` and were able to guess the value of `<UID>`. UID was generated once on startup, was chosen using `Math.random()` and had a keyspace of roughly 4 billion, so within the realm of an online attack. Here's a simple example that will cause `console.log()` to be called when the `serialize()`d version is `eval()`d eval('('+ serialize({"foo": /1" + console.log(1)/i, "bar": '"@__R-<UID>-0__@'}) + ')'); Where `<UID>` is the guessed `UID`. This fixes the issue by ensuring that placeholders are not preceded by a backslash. We also switch to a higher entropy `UID` to prevent people from guessing it. Co-authored-by: Jordan Milne <jmilne@uber.com> Co-authored-by: Ryan Siebert <ryan.siebert@energyaustralia.com.au>

Author: 3 people

index.js

@@ -6,9 +6,12 @@ See the accompanying LICENSE file for terms.
 
 'use strict';
 
+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|D|M|S|U|I)-' + UID + '-(\\d+)__@"', 'g');
+var UID_LENGTH = 16;
+var UID = generateUID();
+var PLACE_HOLDER_REGEXP = new RegExp('(\\\\)?"@__(F|R|D|M|S|U|I)-' + UID + '-(\\d+)__@"', 'g');
 
 var IS_NATIVE_CODE_REGEXP = /\{\s*\[native code\]\s*\}/g;
 var IS_PURE_FUNCTION = /function.*?\(/;
@@ -31,6 +34,15 @@ 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 deleteFunctions(obj){
  var functionKeys = [];
  for (var key in obj) {
@@ -187,7 +199,14 @@ module.exports = function serialize(obj, options) {
  // Replaces all occurrences of function, regexp, date, map and set 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, type, valueIndex) {
+ return str.replace(PLACE_HOLDER_REGEXP, 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;
+ }
+
  if (type === 'D') {
  return "new Date(\"" + dates[valueIndex].toISOString() + "\")";
  }

package-lock.json

@@ -29,5 +29,8 @@
  "chai": "^4.1.0",
  "mocha": "^7.0.0",
  "nyc": "^15.0.0"
+ },
+ "dependencies": {
+ "randombytes": "^2.1.0"
  }
 }

package.json

@@ -1,9 +1,23 @@
 /* global describe, it, beforeEach */
 'use strict';
 
+// temporarily monkeypatch `crypto.randomBytes` so we'll have a
+// predictable UID for our tests
+var crypto = require('crypto');
+var oldRandom = crypto.randomBytes;
+crypto.randomBytes = function(len, cb) {
+ var buf = Buffer.alloc(len);
+ buf.fill(0x00);
+ if (cb)
+ cb(null, buf);
+ return buf;
+};
+
 var serialize = require('../../'),
  expect = require('chai').expect;
 
+crypto.randomBytes = oldRandom;
+
 describe('serialize( obj )', function () {
  it('should be a function', function () {
  expect(serialize).to.be.a('function');
@@ -493,4 +507,17 @@ describe('serialize( obj )', function () {
  expect(serialize([1], 2)).to.equal('[\n 1\n]');
  });
  });
+
+ 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-0000000000000000-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);
+ });
+ });
+
 });