🔒️ Stop json0 ops bypassing the prototype pollution guard

lib/ot.js

@@ -108,18 +108,9 @@ function applyOpEdit(snapshot, edit) {
   var type = types.map[snapshot.type];
   if (!type) return new ShareDBError(ERROR_CODE.ERR_DOC_TYPE_NOT_RECOGNIZED, 'Unknown type');
 
-  if (type.name === 'json0' && Array.isArray(edit)) {
-    for (var i = 0; i < edit.length; i++) {
-      var opComponent = edit[i];
-      if (Array.isArray(opComponent.p)) {
-        for (var j = 0; j < opComponent.p.length; j++) {
-          var pathSegment = opComponent.p[j];
-          if (util.isDangerousProperty(pathSegment)) {
-            return new ShareDBError(ERROR_CODE.ERR_OT_OP_NOT_APPLIED, 'Invalid path segment');
-          }
-        }
-      }
-    }
+  if (type.name === 'json0') {
+    var pathError = checkJson0OpPaths(edit);
+    if (pathError) return pathError;
   }
 
   try {
@@ -129,6 +120,25 @@ function applyOpEdit(snapshot, edit) {
   }
 }
 
+// ot-json0 walks ops with .length and numeric indexing, so it treats array-like
+// objects as ops too, and it coerces .length. This traversal has to match it
+// exactly: anything ot-json0 will apply has to be checked here.
+// See GHSA-9rqw-j2q5-gg2g
+function checkJson0OpPaths(op) {
+  if (op == null) return;
+  for (var i = 0; i < op.length; i++) {
+    var component = op[i];
+    // ot-json0's checkValidOp() rejects the whole op before applying any of it,
+    // so there is nothing beyond this component left to check
+    if (!component || typeof component !== 'object' || !Array.isArray(component.p)) return;
+    for (var j = 0; j < component.p.length; j++) {
+      if (util.isDangerousProperty(component.p[j])) {
+        return new ShareDBError(ERROR_CODE.ERR_OT_OP_NOT_APPLIED, 'Invalid path segment');
+      }
+    }
+  }
+}
+
 exports.transform = function(type, op, appliedOp) {
   // There are 16 cases this function needs to deal with - which are all the
   // combinations of create/delete/op/noop from both op and appliedOp
@@ -178,6 +188,13 @@ exports.applyOps = function(snapshot, ops, options) {
   options = options || {};
   for (var index = 0; index < ops.length; index++) {
     var op = ops[index];
+    // normalizeLegacyJson0Ops() applies op components itself, so paths have to
+    // be checked before it runs, not just in exports.apply()
+    var type = types.map[snapshot.type];
+    if (type && type.name === 'json0' && 'op' in op) {
+      var pathError = checkJson0OpPaths(op.op);
+      if (pathError) return pathError;
+    }
     if (options._normalizeLegacyJson0Ops) {
       try {
         normalizeLegacyJson0Ops(snapshot, op);

lib/projections.js

@@ -58,7 +58,7 @@ function projectEdit(fields, op) {
       }
     } else {
       // The path has a first element. Just check it against the fields.
-      if (fields[path[0]]) {
+      if (util.hasOwn(fields, path[0])) {
         result.push(c);
       }
     }
@@ -91,7 +91,7 @@ function isSnapshotAllowed(fields, snapshot) {
     return false;
   }
   for (var k in snapshot.data) {
-    if (!fields[k]) return false;
+    if (!util.hasOwn(fields, k)) return false;
   }
   return true;
 }
@@ -101,7 +101,7 @@ function isEditAllowed(fields, op) {
     var c = op[i];
     if (c.p.length === 0) {
       return false;
-    } else if (!fields[c.p[0]]) {
+    } else if (!util.hasOwn(fields, c.p[0])) {
       return false;
     }
   }

lib/util.js

@@ -100,14 +100,13 @@ exports.clone = function(obj) {
   return (obj === undefined) ? undefined : JSON.parse(JSON.stringify(obj));
 };
 
+// A null prototype makes '__proto__' an ordinary own key, so it can be in the map
 var objectProtoPropNames = Object.create(null);
 Object.getOwnPropertyNames(Object.prototype).forEach(function(prop) {
-  if (prop !== '__proto__') {
-    objectProtoPropNames[prop] = true;
-  }
+  objectProtoPropNames[prop] = true;
 });
 exports.isDangerousProperty = function(propName) {
-  return propName === '__proto__' || objectProtoPropNames[propName];
+  return objectProtoPropNames[propName] === true;
 };
 
 try {

test/client/doc.js

@@ -682,6 +682,10 @@ describe('Doc', function() {
       });
     }
 
+    afterEach(function() {
+      delete Object.prototype.polluted;
+    });
+
     ['__proto__', 'constructor'].forEach(function(badProp) {
       it('Rejects ops with collection ' + badProp, function(done) {
         var collectionName = badProp;
@@ -755,6 +759,46 @@ describe('Doc', function() {
         });
       });
     });
+
+    // ot-json0 walks ops with .length and numeric indexing, so it applies an
+    // array-like object as if it were an op
+    [
+      {
+        name: 'an array-like op',
+        op: {0: {p: ['__proto__', 'polluted'], oi: 'oops'}, length: 1}
+      },
+      {
+        name: 'ops with a path segment that is not a string',
+        op: [{p: [['__proto__'], 'polluted'], oi: 'oops'}]
+      }
+    ].forEach(function(test) {
+      it('Rejects ' + test.name, function(done) {
+        var connection = this.connection;
+        var collectionName = 'test-collection';
+        var docId = 'test-doc';
+        connection.get(collectionName, docId).create({id: docId}, function(err) {
+          if (err) {
+            return done(err);
+          }
+          expectReceiveError(connection, collectionName, docId, 'Invalid path segment', function(error) {
+            if (error) {
+              return done(error);
+            }
+            expect({}.polluted).to.equal(undefined);
+            done();
+          });
+          connection.send({
+            a: 'op',
+            c: collectionName,
+            d: docId,
+            v: 1,
+            seq: connection.seq++,
+            x: {},
+            op: test.op
+          });
+        });
+      });
+    });
   });
 
   describe('toSnapshot', function() {

test/ot.js

@@ -125,6 +125,68 @@ describe('ot', function() {
     });
   });
 
+  describe('json0 op validation', function() {
+    var snapshot;
+
+    beforeEach(function() {
+      snapshot = {v: 6, type: type.uri, data: {colour: 'blue'}};
+    });
+
+    afterEach(function() {
+      delete Object.prototype.polluted;
+    });
+
+    it('does not pollute the prototype through an array-like op', function() {
+      var op = {0: {p: ['__proto__', 'polluted'], oi: 'yes'}, length: 1};
+      var error = ot.apply(snapshot, {v: 6, op: op});
+      expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+      expect(error.message).to.equal('Invalid path segment');
+      expect({}.polluted).to.equal(undefined);
+    });
+
+    it('does not pollute the prototype through an array-like op with a string length', function() {
+      var op = {0: {p: ['__proto__', 'polluted'], oi: 'yes'}, length: '1'};
+      var error = ot.apply(snapshot, {v: 6, op: op});
+      expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+      expect(error.message).to.equal('Invalid path segment');
+      expect({}.polluted).to.equal(undefined);
+    });
+
+    it('does not pollute the prototype through a path segment that is not a string', function() {
+      var error = ot.apply(snapshot, {v: 6, op: [{p: [['__proto__'], 'polluted'], oi: 'yes'}]});
+      expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+      expect(error.message).to.equal('Invalid path segment');
+      expect({}.polluted).to.equal(undefined);
+    });
+
+    ['__proto__', 'constructor'].forEach(function(badProp) {
+      it('rejects ' + badProp + ' as a path segment', function() {
+        var error = ot.apply(snapshot, {v: 6, op: [{p: [badProp, 'polluted'], oi: 'yes'}]});
+        expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+        expect(error.message).to.equal('Invalid path segment');
+        expect({}.polluted).to.equal(undefined);
+      });
+    });
+
+    it('returns an error for an op component that is not an object', function() {
+      var error;
+      expect(function() {
+        error = ot.apply(snapshot, {v: 6, op: [null]});
+      }).to.not.throw();
+      expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+    });
+
+    it('returns an error for an array-like op whose components are missing', function() {
+      var error = ot.apply(snapshot, {v: 6, op: {length: 1e9}});
+      expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+    });
+
+    it('accepts an op that ot-json0 treats as a no-op', function() {
+      expect(ot.apply(snapshot, {v: 6, op: {p: ['colour'], oi: 'red'}})).equal();
+      expect(snapshot).to.eql({v: 7, type: type.uri, data: {colour: 'blue'}});
+    });
+  });
+
   describe('no-op', function() {
     it('works on existing docs', function() {
       var doc = {v: 6, type: type.uri, data: 'Hi'};
@@ -374,6 +436,55 @@ describe('ot', function() {
 
   describe('applyOps', function() {
     describe('with normalization turned on', function() {
+      afterEach(function() {
+        delete Object.prototype.polluted;
+      });
+
+      it('does not pollute the prototype while normalizing a legacy op', function() {
+        var snapshot = {
+          type: 'http://sharejs.org/types/JSONv0',
+          data: {title: 'Wee Free Men'}
+        };
+
+        var ops = [
+          {
+            v: 1,
+            op: [
+              {p: ['__proto__', 'polluted'], oi: 'yes'},
+              {p: ['title'], od: 'Wee Free Men', oi: 'Nation'}
+            ]
+          }
+        ];
+
+        var error = ot.applyOps(snapshot, ops, {
+          _normalizeLegacyJson0Ops: true
+        });
+        expect(error.code).to.equal(ERROR_CODE.ERR_OT_OP_NOT_APPLIED);
+        expect(error.message).to.equal('Invalid path segment');
+        expect({}.polluted).to.equal(undefined);
+      });
+
+      it('replays a legacy op that ot-json0 treats as a no-op', function() {
+        var snapshot = {
+          type: 'http://sharejs.org/types/JSONv0',
+          data: {title: 'Wee Free Men'}
+        };
+
+        var ops = [
+          {
+            v: 1,
+            op: {p: ['title'], od: 'Wee Free Men', oi: 'Nation'}
+          }
+        ];
+
+        var error = ot.applyOps(snapshot, ops, {
+          _normalizeLegacyJson0Ops: true
+        });
+        expect(error).to.be.undefined;
+        expect(snapshot.data).to.eql({title: 'Wee Free Men'});
+        expect(snapshot.v).to.equal(2);
+      });
+
       it('applies an op to a snapshot', function() {
         var snapshot = {
           type: 'http://sharejs.org/types/JSONv0',

test/projections.js

@@ -385,4 +385,20 @@ describe('projection utility methods', function() {
       );
     });
   });
+
+  describe('field names inherited from Object.prototype', function() {
+    ['__proto__', 'constructor', 'toString'].forEach(function(badProp) {
+      it('does not treat ' + badProp + ' as a projected field', function() {
+        var op = {op: [{p: [badProp, 'x'], oi: 'oops'}]};
+        expect(projections.isOpAllowed(null, {x: true}, op)).equal(false);
+        projections.projectOp({x: true}, op);
+        expect(op).eql({op: []});
+      });
+
+      it('does not treat ' + badProp + ' as an allowed snapshot field', function() {
+        var data = JSON.parse('{"' + badProp + '": "oops"}');
+        expect(projections.isSnapshotAllowed({x: true}, {type: type, data: data})).equal(false);
+      });
+    });
+  });
 });