fix: Object update returns stored field values to a caller without read access (GHSA-2rcp-jc67-vq74)

spec/vulnerabilities.spec.js

@@ -10009,6 +10009,188 @@ describe('Vulnerabilities', () => {
     });
   });
 
+  describe('(GHSA-2rcp-jc67-vq74) Save response returns the stored parent of a dot notation key to a caller without read access', () => {
+    const headers = {
+      'X-Parse-Application-Id': 'test',
+      'X-Parse-REST-API-Key': 'rest',
+      'Content-Type': 'application/json',
+    };
+
+    describe('class without get permission', () => {
+      let reader;
+      let obj;
+
+      beforeEach(async () => {
+        reader = await Parse.User.signUp('reader', 'password');
+        const schema = new Parse.Schema('WriteOnly');
+        schema.addObject('field');
+        schema.setCLP({
+          find: { [reader.id]: true },
+          get: { [reader.id]: true },
+          count: { [reader.id]: true },
+          create: {},
+          update: { '*': true },
+          delete: {},
+          addField: {},
+        });
+        await schema.save();
+        obj = await new Parse.Object('WriteOnly').save(
+          { field: { sub: 'stored-sub', other: 'stored-other' } },
+          { useMasterKey: true }
+        );
+      });
+
+      const update = (body, extraHeaders = {}) =>
+        request({
+          method: 'PUT',
+          url: `${Parse.serverURL}/classes/WriteOnly/${obj.id}`,
+          headers: { ...headers, ...extraHeaders },
+          body: JSON.stringify(body),
+        });
+
+      it('does not return the stored parent of a dot notation key with afterSave', async () => {
+        Parse.Cloud.afterSave('WriteOnly', () => {});
+        const get = await request({
+          url: `${Parse.serverURL}/classes/WriteOnly/${obj.id}`,
+          headers,
+        }).catch(e => e);
+        expect(get.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
+
+        const response = await update({ 'field.added': 'x' });
+        expect(response.data.field).toBeUndefined();
+        expect(JSON.stringify(response.data)).not.toContain('stored-');
+        await obj.fetch({ useMasterKey: true });
+        expect(obj.get('field')).toEqual({ sub: 'stored-sub', other: 'stored-other', added: 'x' });
+      });
+
+      it('returns only the operation result for a dot notation operation with afterSave', async () => {
+        Parse.Cloud.afterSave('WriteOnly', () => {});
+        const response = await update({ 'field.count': { __op: 'Increment', amount: 1 } });
+        expect(response.data.field).toEqual({ count: 1 });
+      });
+
+      it('does not return the stored parent of a dot notation key changed by afterSave', async () => {
+        Parse.Cloud.afterSave('WriteOnly', ({ object }) => {
+          object.set('field.audit', 'audited');
+        });
+        const response = await update({ 'field.added': 'x' });
+        expect(JSON.stringify(response.data)).not.toContain('stored-');
+      });
+
+      it('does not return the stored parent of a dot notation key with beforeSave', async () => {
+        Parse.Cloud.beforeSave('WriteOnly', () => {});
+        const response = await update({ 'field.added': 'x' });
+        expect(response.data.field).toBeUndefined();
+        expect(JSON.stringify(response.data)).not.toContain('stored-');
+      });
+
+      it('keeps an object returned by afterSave', async () => {
+        Parse.Cloud.afterSave('WriteOnly', () => ({ field: 'returned' }));
+        const response = await update({ 'field.added': 'x' });
+        expect(response.data.field).toBe('returned');
+      });
+
+      it('does not return the stored ACL for a dot notation ACL key with beforeSave', async () => {
+        const acl = new Parse.ACL();
+        acl.setPublicWriteAccess(true);
+        acl.setReadAccess('storedReaderId', true);
+        obj.setACL(acl);
+        await obj.save(null, { useMasterKey: true });
+        Parse.Cloud.beforeSave('WriteOnly', () => {});
+        const response = await update({ 'ACL.newUserId': { read: true } });
+        expect(response.data.ACL).toBeUndefined();
+        expect(JSON.stringify(response.data)).not.toContain('storedReaderId');
+      });
+
+      it('does not return the stored parent of a dot notation key in a batch request', async () => {
+        Parse.Cloud.afterSave('WriteOnly', () => {});
+        const response = await request({
+          method: 'POST',
+          url: `${Parse.serverURL}/batch`,
+          headers,
+          body: JSON.stringify({
+            requests: [
+              { method: 'PUT', path: `/1/classes/WriteOnly/${obj.id}`, body: { 'field.added': 'x' } },
+            ],
+          }),
+        });
+        expect(response.data[0].success.field).toBeUndefined();
+        expect(JSON.stringify(response.data)).not.toContain('stored-');
+      });
+
+      it('does not read the object for a dot notation operation without triggers', async () => {
+        const DatabaseController = require('../lib/Controllers/DatabaseController');
+        const find = spyOn(DatabaseController.prototype, 'find').and.callThrough();
+        const response = await update({ 'field.count': { __op: 'Increment', amount: 1 } });
+        expect(response.data.field).toEqual({ count: 1 });
+        expect(find.calls.allArgs().filter(([className]) => className === 'WriteOnly').length).toBe(0);
+      });
+
+      for (const auth of ['session with get permission', 'master', 'maintenance']) {
+        it(`returns the parent of a dot notation key for ${auth} auth`, async () => {
+          Parse.Cloud.afterSave('WriteOnly', () => {});
+          const authHeaders = {
+            'session with get permission': { 'X-Parse-Session-Token': reader.getSessionToken() },
+            master: { 'X-Parse-Master-Key': 'test' },
+            maintenance: { 'X-Parse-Maintenance-Key': 'testing' },
+          }[auth];
+          const response = await update({ 'field.added': 'x' }, authHeaders);
+          expect(response.data.field).toEqual({
+            sub: 'stored-sub',
+            other: 'stored-other',
+            added: 'x',
+          });
+        });
+      }
+    });
+
+    it('does not return the stored parent of a dot notation key for an object without read access', async () => {
+      const writer = await Parse.User.signUp('writer', 'password');
+      const acl = new Parse.ACL();
+      acl.setWriteAccess(writer, true);
+      acl.setReadAccess('otherUserId', true);
+      const obj = new Parse.Object('WriteOnlyAcl');
+      obj.setACL(acl);
+      await obj.save({ field: { sub: 'stored-sub' } }, { useMasterKey: true });
+      Parse.Cloud.afterSave('WriteOnlyAcl', () => {});
+      const sessionHeaders = { ...headers, 'X-Parse-Session-Token': writer.getSessionToken() };
+      const get = await request({
+        url: `${Parse.serverURL}/classes/WriteOnlyAcl/${obj.id}`,
+        headers: sessionHeaders,
+      }).catch(e => e);
+      expect(get.data.code).toBe(Parse.Error.OBJECT_NOT_FOUND);
+
+      const response = await request({
+        method: 'PUT',
+        url: `${Parse.serverURL}/classes/WriteOnlyAcl/${obj.id}`,
+        headers: sessionHeaders,
+        body: JSON.stringify({ 'field.added': 'x' }),
+      });
+      expect(response.data.field).toBeUndefined();
+      expect(JSON.stringify(response.data)).not.toContain('stored-sub');
+    });
+
+    it('does not return a protected parent of a dot notation key when protectedFieldsTriggerExempt is true', async () => {
+      await reconfigureServer({
+        protectedFields: { MyClass: { '*': ['secretField'] } },
+        protectedFieldsTriggerExempt: true,
+      });
+      const obj = await new Parse.Object('MyClass').save(
+        { secretField: { sub: 'stored-sub' } },
+        { useMasterKey: true }
+      );
+      Parse.Cloud.afterSave('MyClass', () => {});
+      const response = await request({
+        method: 'PUT',
+        url: `${Parse.serverURL}/classes/MyClass/${obj.id}`,
+        headers,
+        body: JSON.stringify({ 'secretField.added': 'x' }),
+      });
+      expect(response.data.secretField).toBeUndefined();
+      expect(JSON.stringify(response.data)).not.toContain('stored-sub');
+    });
+  });
+
   describe('(GHSA-rx55-fw4j-pvpx) Dot notation key corrupts shared runtime object', () => {
     const headers = {
       'X-Parse-Application-Id': 'test',

src/RestWrite.js

@@ -76,6 +76,8 @@ function RestWrite(config, auth, className, query, data, originalData, context,
   // copy
   this.query = structuredClone(query);
   this.data = structuredClone(data);
+  // Before triggers change the data
+  this.dotNotationKeys = Object.keys(this.data || {}).filter(key => key.includes('.'));
   // We never change originalData, so we do not need a deep copy
   this.originalData = originalData;
 
@@ -168,7 +170,7 @@ RestWrite.prototype.execute = function () {
       return this.cleanUserAuthData();
     })
     .then(() => {
-      return this.filterRevertedFieldsInResponse();
+      return this.filterUnreadableFieldsInResponse();
     })
     .then(() => {
       return this.filterProtectedFieldsInResponse();
@@ -1825,6 +1827,8 @@ RestWrite.prototype.runDatabaseOperation = function () {
         })
         .then(response => {
           response.updatedAt = this.updatedAt;
+          // The response without trigger changes
+          this.databaseResponse = { ...response };
           this._updateResponseWithData(response, this.data);
           this.response = { response };
         });
@@ -1986,7 +1990,9 @@ RestWrite.prototype.runAfterSaveTrigger = function () {
     .then(result => {
       const jsonReturned = result && !result._toFullJSON;
       if (jsonReturned) {
+        // The returned object is the response as is
         this.pendingOps.operations = {};
+        this.dotNotationKeys = [];
         this.response.response = result;
       } else {
         this.response.response = this._updateResponseWithData(
@@ -2233,22 +2239,32 @@ RestWrite.prototype._restoreRevertedPath = function (response, writeData, writte
   response[target] = value === undefined ? { __op: 'Delete' } : value;
 };
 
-// Returns reverted fields of an update only as the caller can read them
-RestWrite.prototype.filterRevertedFieldsInResponse = async function () {
+// Returns stored values in an update response only as the caller can read them
+RestWrite.prototype.filterUnreadableFieldsInResponse = async function () {
   const response = this.response?.response;
-  if (!this.query || !response || !this.pendingOps.operations) {
+  if (!this.query || !response) {
     return;
   }
   if (this.auth.isMaster || this.auth.isMaintenance) {
     return;
   }
-  const stateController = Parse.CoreManager.getObjectStateController();
-  const [pending] = stateController.getPendingOps(this.pendingOps.identifier);
-  const reverted = Object.keys(this.pendingOps.operations).filter(key => !pending[key]);
+  let reverted = [];
+  if (this.pendingOps.operations) {
+    const stateController = Parse.CoreManager.getObjectStateController();
+    const [pending] = stateController.getPendingOps(this.pendingOps.identifier);
+    reverted = Object.keys(this.pendingOps.operations).filter(key => !pending[key]);
+  }
   const keys = Object.keys(response).filter(key =>
     reverted.some(path => path === key || path.startsWith(`${key}.`))
   );
-  if (!keys.length) {
+  // Parents of dot notation keys beyond the database response
+  const parents = Object.keys(response).filter(
+    key =>
+      !keys.includes(key) &&
+      this.dotNotationKeys.some(path => path.startsWith(`${key}.`)) &&
+      !util.isDeepStrictEqual(response[key], this.databaseResponse?.[key])
+  );
+  if (!keys.length && !parents.length) {
     return;
   }
   let view;
@@ -2295,6 +2311,17 @@ RestWrite.prototype.filterRevertedFieldsInResponse = async function () {
       response[key] = value;
     }
   }
+  for (const key of parents) {
+    if (getOwnPath(view, key.split('.')) !== undefined) {
+      continue;
+    }
+    // As returned without triggers
+    if (hasOwn(this.databaseResponse, key)) {
+      response[key] = this.databaseResponse[key];
+    } else {
+      delete response[key];
+    }
+  }
 };
 
 RestWrite.prototype._updateResponseWithData = function (response, data, isFinalObject = false) {