diff --git a/spec/Utils.spec.js b/spec/Utils.spec.js index 90891ede62..f44882e0ad 100644 --- a/spec/Utils.spec.js +++ b/spec/Utils.spec.js @@ -468,4 +468,20 @@ describe('Utils', () => { }); } }); + + describe('safeToJSON', () => { + it('returns the serialized object', () => { + const object = { toJSON: () => ({ a: 1 }) }; + expect(Utils.safeToJSON(object)).toEqual({ a: 1 }); + }); + + it('returns undefined when serialization throws', () => { + const object = { + toJSON: () => { + throw new Error('Cannot create a pointer to an unsaved ParseObject'); + }, + }; + expect(Utils.safeToJSON(object)).toBeUndefined(); + }); + }); }); diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index 71a1168c9c..ce965e6919 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -11030,4 +11030,223 @@ describe('Vulnerabilities', () => { expect((await readInstallation(own)).localeIdentifier).toBe('self'); }); }); + + describe('(GHSA-wc3c-6843-q7mv) Dot notation key below a pointer field with an afterSave trigger', () => { + const serverURL = 'http://localhost:8378/1'; + const masterHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-Master-Key': 'test', + 'Content-Type': 'application/json', + }; + const clientHeaders = { + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + 'Content-Type': 'application/json', + }; + + let rejections; + const onRejection = reason => rejections.push(reason); + beforeEach(() => { + rejections = []; + process.on('unhandledRejection', onRejection); + }); + afterEach(() => { + process.removeListener('unhandledRejection', onRejection); + }); + + // Resolves with the response, or with { hung: true } if the request does not complete in time. + const within = (promise, ms = 7000) => + Promise.race([ + promise.then( + r => ({ status: r.status, data: r.data }), + e => ({ status: e.status, data: e.data }) + ), + new Promise(resolve => setTimeout(() => resolve({ hung: true }), ms)), + ]); + + async function makeTarget() { + const target = new Parse.Object('Target'); + await target.save({ n: 1 }, { useMasterKey: true }); + return target; + } + async function makeHolder(fields) { + const holder = new Parse.Object('Holder'); + await holder.save(fields, { useMasterKey: true }); + return holder; + } + + it_only_db('mongo')('completes a dot notation delete of a pointer id', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ owner: target }); + let seenOwnerId = 'not-called'; + Parse.Cloud.afterSave('Holder', req => { + const owner = req.object.get('owner'); + seenOwnerId = owner ? owner.id : null; + }); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'owner.id': { __op: 'Delete' } }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(seenOwnerId).toBe(target.id); + expect(rejections).toEqual([]); + }); + + it_only_db('mongo')('completes a dot notation null on a pointer id', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ owner: target }); + let seenOwnerId = 'not-called'; + Parse.Cloud.afterSave('Holder', req => { + const owner = req.object.get('owner'); + seenOwnerId = owner ? owner.id : null; + }); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'owner.id': null }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(seenOwnerId).toBe(target.id); + expect(rejections).toEqual([]); + }); + + it('completes when beforeSave reverts a dot notation key below a pointer', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ owner: target }); + Parse.Cloud.beforeSave('Holder', ({ object }) => { + object.revert('owner.id'); + }); + let seenOwnerId = 'not-called'; + Parse.Cloud.afterSave('Holder', req => { + const owner = req.object.get('owner'); + seenOwnerId = owner ? owner.id : null; + }); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'owner.id': { __op: 'Delete' } }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(seenOwnerId).toBe(target.id); + expect(rejections).toEqual([]); + }); + + it('completes a dot notation delete of a nested pointer id', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ meta: { ptr: target.toPointer() } }); + Parse.Cloud.afterSave('Holder', () => {}); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'meta.ptr.id': { __op: 'Delete' } }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(rejections).toEqual([]); + }); + + it('still deletes a dot notation key below a plain object field', async () => { + const holder = await makeHolder({ meta: { note: 'keep', other: 'x' } }); + let seenMeta = 'not-called'; + Parse.Cloud.afterSave('Holder', req => { + seenMeta = req.object.get('meta'); + }); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'meta.note': { __op: 'Delete' } }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(seenMeta).toEqual({ other: 'x' }); + expect(rejections).toEqual([]); + }); + + it_only_db('mongo')('completes a dot notation delete of a pointer id in a batch request', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ owner: target }); + Parse.Cloud.afterSave('Holder', () => {}); + const result = await within( + request({ + method: 'POST', + url: `${serverURL}/batch`, + headers: masterHeaders, + body: JSON.stringify({ + requests: [ + { + method: 'PUT', + path: `/1/classes/Holder/${holder.id}`, + body: { 'owner.id': { __op: 'Delete' } }, + }, + ], + }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(result.data[0].success).toBeDefined(); + expect(rejections).toEqual([]); + }); + + it_only_db('mongo')('completes for a non-master caller on a publicly writable object', async () => { + const target = await makeTarget(); + const holder = new Parse.Object('Holder'); + holder.set('owner', target); + holder.setACL(new Parse.ACL({ '*': { read: true, write: true } })); + await holder.save(null, { useMasterKey: true }); + Parse.Cloud.afterSave('Holder', () => {}); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: clientHeaders, + body: JSON.stringify({ 'owner.id': { __op: 'Delete' } }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(rejections).toEqual([]); + }); + + it_only_db('mongo')('passes the stored pointer to the afterSave trigger unchanged', async () => { + const target = await makeTarget(); + const holder = await makeHolder({ owner: target }); + let ownerPointer = 'not-called'; + Parse.Cloud.afterSave('Holder', req => { + const owner = req.object.get('owner'); + ownerPointer = owner ? owner.toPointer() : null; + }); + const result = await within( + request({ + method: 'PUT', + url: `${serverURL}/classes/Holder/${holder.id}`, + headers: masterHeaders, + body: JSON.stringify({ 'owner.id': { __op: 'Delete' }, label: 'x' }), + }) + ); + expect(result.hung).toBeUndefined(); + expect(result.status).toBe(200); + expect(ownerPointer).toEqual({ __type: 'Pointer', className: 'Target', objectId: target.id }); + expect(rejections).toEqual([]); + }); + }); }); diff --git a/src/RestWrite.js b/src/RestWrite.js index 29f4e1c413..dc9211445a 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -2036,6 +2036,21 @@ RestWrite.prototype.sanitizedData = function () { return Parse._decode(undefined, data); }; +// A raw view of the object's fields (stored values overlaid with whole-field +// writes in this request) used to resolve whether a dot notation key targets a +// pointer's object ID. +RestWrite.prototype._dotNotationFieldRoot = function () { + const root = Object.assign({}, this.originalData); + if (this.data) { + for (const key of Object.keys(this.data)) { + if (key.indexOf('.') < 0) { + root[key] = this.data[key]; + } + } + } + return root; +}; + // Returns an updated copy of the object RestWrite.prototype.buildParseObjects = function () { const extraData = { className: this.className, objectId: this.query?.objectId }; @@ -2062,11 +2077,14 @@ RestWrite.prototype.buildParseObjects = function () { } } const updatedObject = triggers.inflate(extraData, this.originalData); + const pointerRoot = this._dotNotationFieldRoot(); Object.keys(this.data).reduce(function (data, key) { if (key.indexOf('.') > 0) { // Null and undefined are set at their own dot notation path, like an operation if (data[key] == null || typeof data[key].__op === 'string') { - if (!readOnlyAttributes.includes(key)) { + // A dot notation key that removes a pointer's object ID would leave the + // pointer unserializable and break the trigger object, so skip it. + if (!readOnlyAttributes.includes(key) && !targetsPointerId(pointerRoot, key)) { // The SDK key name rules apply to top-level fields only updatedObject.set(key, data[key], { ignoreValidation: true }); } @@ -2159,6 +2177,22 @@ const getOwnPath = (object, path) => { return value; }; +// Whether a dot notation key targets a pointer's object ID (`id`/`objectId`). +// Removing it leaves the pointer unserializable (`toPointer` throws), which +// corrupts the trigger object; other sub-keys below a pointer are left to the +// existing handling. +const targetsPointerId = (root, key) => { + const segments = key.split('.'); + for (let depth = 1; depth < segments.length; depth++) { + const parent = getOwnPath(root, segments.slice(0, depth)); + if (parent !== null && typeof parent === 'object' && parent.__type === 'Pointer') { + const next = segments[depth]; + return next === 'id' || next === 'objectId'; + } + } + return false; +}; + // Restores a reverted dot notation key in the response RestWrite.prototype._restoreRevertedPath = function (response, writeData, written, key, finalObject) { // A write at or beneath the key governs diff --git a/src/Utils.js b/src/Utils.js index 0fbcb1f0d5..30dda64fae 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -593,6 +593,22 @@ class Utils { } return filename.substring(filename.lastIndexOf('.') + 1); } + + /** + * @function safeToJSON + * @description Serializes a Parse object, returning `undefined` if serialization + * throws (for example a pointer left without an object ID by a dot notation write). + * Use where a failed serialization must not interrupt control flow, such as logging. + * @param {object} object The Parse object to serialize. + * @returns {object|undefined} The serialized object, or `undefined` on failure. + */ + static safeToJSON(object) { + try { + return object.toJSON(); + } catch { + return undefined; + } + } } module.exports = Utils; diff --git a/src/triggers.js b/src/triggers.js index f34cfcc7ec..16cce23a84 100644 --- a/src/triggers.js +++ b/src/triggers.js @@ -953,7 +953,7 @@ export function maybeRunTrigger( logTriggerSuccessBeforeHook( triggerType, parseObject.className, - parseObject.toJSON(), + Utils.safeToJSON(parseObject), object, auth, triggerType.startsWith('after') @@ -974,7 +974,7 @@ export function maybeRunTrigger( logTriggerErrorBeforeHook( triggerType, parseObject.className, - parseObject.toJSON(), + Utils.safeToJSON(parseObject), auth, error, config.logLevels.triggerBeforeError @@ -1005,7 +1005,7 @@ export function maybeRunTrigger( logTriggerAfterHook( triggerType, parseObject.className, - parseObject.toJSON(), + Utils.safeToJSON(parseObject), auth, config.logLevels.triggerAfter );