From 46dd037d6331f311270bb29e6cf1397999dc7448 Mon Sep 17 00:00:00 2001 From: Alec Gibson <12036746+alecgibson@users.noreply.github.com> Date: Wed, 6 Dec 2023 11:08:28 +0000 Subject: [PATCH] Guard against prototype pollution in json0 `json0.apply` has a prototype pollution security issue, where applying ops with path segments that match prototype property names can clobber said prototype properties. This can cause a DoS by crashing a server running json0. (We've just released safeguards in sharedb, which still uses json0 as the default type.) This fixes the issue by throwing an error in `json0.apply` when encountering a path segment that matches the name of a prototype property. --- lib/json0.js | 7 ++++++- test/json0.coffee | 10 ++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/lib/json0.js b/lib/json0.js index 4c92407..4547e83 100644 --- a/lib/json0.js +++ b/lib/json0.js @@ -34,6 +34,8 @@ var isObject = function(obj) { return (!!obj) && (obj.constructor === Object); }; +var hasOwn = Object.hasOwn || Object.prototype.hasOwnProperty.call; + /** * Clones the passed object using JSON serialization (which is slow). * @@ -57,7 +59,7 @@ var json = { // You can register another OT type as a subtype in a JSON document using // the following function. This allows another type to handle certain // operations instead of the builtin JSON type. -var subtypes = {}; +var subtypes = Object.create(null); json.registerSubtype = function(subtype) { subtypes[subtype.name] = subtype; }; @@ -177,6 +179,9 @@ json.apply = function(snapshot, op) { for (var j = 0; j < c.p.length; j++) { var p = c.p[j]; + if (p in elem && !hasOwn(elem, p)) + throw new Error('Path invalid'); + parent = elem; parentKey = key; elem = elem[key]; diff --git a/test/json0.coffee b/test/json0.coffee index 05294e6..85cbf28 100644 --- a/test/json0.coffee +++ b/test/json0.coffee @@ -452,6 +452,16 @@ genTests = (type) -> assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'c', od: 'b'}] assert.throws -> type.apply {x:'a'}, [{p:['x'], oi: 'b'}] + it 'disallows reassignment of special JS property names', -> + assert.throws -> type.apply {x:'a'}, [{p:['__proto__'], oi:'oops'}] + assert.throws -> type.apply {x:{y:'a'}}, [{p:['x', '__proto__'], oi:'oops'}] + assert.throws -> type.apply {x:'a'}, [{p:['constructor'], oi:'oops'}] + assert.throws -> type.apply {x:{y:'a'}}, [{p:['x', 'constructor'], oi:'oops'}] + + it 'disallows modification of prototype property objects', -> + obj = {x:'a'} + assert.throws -> type.apply obj, [{p:['toString', 'name'], oi:'oops'}] + it 'throws when the insertion key is a number', -> assert.throws -> type.apply {'1':'a'}, [{p:[2], oi: 'a'}]