From 4c0c7c77b76257878b9bcb05ff9de01c9d790262 Mon Sep 17 00:00:00 2001 From: Manuel <5673677+mtrezza@users.noreply.github.com> Date: Fri, 2 Sep 2022 21:43:31 +0200 Subject: [PATCH] fix: brute force guessing of user sensitive data via search patterns (GHSA-2m6g-crv8-p3c6) (#8146) [skip release] --- spec/RestQuery.spec.js | 73 +++++++++++++++++++++++++++ src/Controllers/DatabaseController.js | 71 +++++++++++++------------- src/RestQuery.js | 27 ++++++++++ 3 files changed, 134 insertions(+), 37 deletions(-) diff --git a/spec/RestQuery.spec.js b/spec/RestQuery.spec.js index deac233915..2e37c96b7c 100644 --- a/spec/RestQuery.spec.js +++ b/spec/RestQuery.spec.js @@ -191,6 +191,79 @@ describe('rest query', () => { expect(result.results.length).toEqual(0); }); + it('query internal field', async () => { + const internalFields = [ + '_email_verify_token', + '_perishable_token', + '_tombstone', + '_email_verify_token_expires_at', + '_failed_login_count', + '_account_lockout_expires_at', + '_password_changed_at', + '_password_history', + ]; + await Promise.all([ + ...internalFields.map(field => + expectAsync(new Parse.Query(Parse.User).exists(field).find()).toBeRejectedWith( + new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid key name: ${field}`) + ) + ), + ...internalFields.map(field => + new Parse.Query(Parse.User).exists(field).find({ useMasterKey: true }) + ), + ]); + }); + + it('query protected field', async () => { + const user = new Parse.User(); + user.setUsername('username1'); + user.setPassword('password'); + await user.signUp(); + const config = Config.get(Parse.applicationId); + const obj = new Parse.Object('Test'); + + obj.set('owner', user); + obj.set('test', 'test'); + obj.set('zip', 1234); + await obj.save(); + + const schema = await config.database.loadSchema(); + await schema.updateClass( + 'Test', + {}, + { + get: { '*': true }, + find: { '*': true }, + protectedFields: { [user.id]: ['zip'] }, + } + ); + await Promise.all([ + new Parse.Query('Test').exists('test').find(), + expectAsync(new Parse.Query('Test').exists('zip').find()).toBeRejectedWith( + new Parse.Error( + Parse.Error.OPERATION_FORBIDDEN, + 'This user is not allowed to query zip on class Test' + ) + ), + ]); + }); + + it('query protected field with matchesQuery', async () => { + const user = new Parse.User(); + user.setUsername('username1'); + user.setPassword('password'); + await user.signUp(); + const test = new Parse.Object('TestObject', { user }); + await test.save(); + const subQuery = new Parse.Query(Parse.User); + subQuery.exists('_perishable_token'); + await expectAsync( + new Parse.Query('TestObject').matchesQuery('user', subQuery).find() + ).toBeRejectedWith( + new Parse.Error(Parse.Error.INVALID_KEY_NAME, 'Invalid key name: _perishable_token') + ); + }); + it('query with wrongly encoded parameter', done => { rest .create(config, nobody, 'TestParameterEncode', { foo: 'bar' }) diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 8e99b29216..25b97d0ec7 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -55,31 +55,27 @@ const transformObjectACL = ({ ACL, ...result }) => { return result; }; -const specialQuerykeys = [ - '$and', - '$or', - '$nor', - '_rperm', - '_wperm', - '_perishable_token', +const specialQueryKeys = ['$and', '$or', '$nor', '_rperm', '_wperm']; +const specialMasterQueryKeys = [ + ...specialQueryKeys, '_email_verify_token', + '_perishable_token', + '_tombstone', '_email_verify_token_expires_at', - '_account_lockout_expires_at', '_failed_login_count', + '_account_lockout_expires_at', + '_password_changed_at', + '_password_history', ]; -const isSpecialQueryKey = key => { - return specialQuerykeys.indexOf(key) >= 0; -}; - -const validateQuery = (query: any): void => { +const validateQuery = (query: any, isMaster: boolean, update: boolean): void => { if (query.ACL) { throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot query on ACL.'); } if (query.$or) { if (query.$or instanceof Array) { - query.$or.forEach(validateQuery); + query.$or.forEach(value => validateQuery(value, isMaster, update)); } else { throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array value.'); } @@ -87,7 +83,7 @@ const validateQuery = (query: any): void => { if (query.$and) { if (query.$and instanceof Array) { - query.$and.forEach(validateQuery); + query.$and.forEach(value => validateQuery(value, isMaster, update)); } else { throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $and format - use an array value.'); } @@ -95,7 +91,7 @@ const validateQuery = (query: any): void => { if (query.$nor) { if (query.$nor instanceof Array && query.$nor.length > 0) { - query.$nor.forEach(validateQuery); + query.$nor.forEach(value => validateQuery(value, isMaster, update)); } else { throw new Parse.Error( Parse.Error.INVALID_QUERY, @@ -115,7 +111,11 @@ const validateQuery = (query: any): void => { } } } - if (!isSpecialQueryKey(key) && !key.match(/^[a-zA-Z][a-zA-Z0-9_\.]*$/)) { + if ( + !key.match(/^[a-zA-Z][a-zA-Z0-9_\.]*$/) && + ((!specialQueryKeys.includes(key) && !isMaster && !update) || + (update && isMaster && !specialMasterQueryKeys.includes(key))) + ) { throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid key name: ${key}`); } }); @@ -208,27 +208,24 @@ const filterSensitiveData = ( perms.protectedFields.temporaryKeys.forEach(k => delete object[k]); } - if (!isUserClass) { - return object; + if (isUserClass) { + object.password = object._hashed_password; + delete object._hashed_password; + delete object.sessionToken; } - object.password = object._hashed_password; - delete object._hashed_password; - - delete object.sessionToken; - if (isMaster) { return object; } - delete object._email_verify_token; - delete object._perishable_token; - delete object._perishable_token_expires_at; - delete object._tombstone; - delete object._email_verify_token_expires_at; - delete object._failed_login_count; - delete object._account_lockout_expires_at; - delete object._password_changed_at; - delete object._password_history; + for (const key in object) { + if (key.charAt(0) === '_') { + delete object[key]; + } + } + + if (!isUserClass) { + return object; + } if (aclGroup.indexOf(object.objectId) > -1) { return object; @@ -515,7 +512,7 @@ class DatabaseController { if (acl) { query = addWriteACL(query, acl); } - validateQuery(query); + validateQuery(query, isMaster, true); return schemaController .getOneSchema(className, true) .catch(error => { @@ -761,7 +758,7 @@ class DatabaseController { if (acl) { query = addWriteACL(query, acl); } - validateQuery(query); + validateQuery(query, isMaster, false); return schemaController .getOneSchema(className) .catch(error => { @@ -1253,7 +1250,7 @@ class DatabaseController { query = addReadACL(query, aclGroup); } } - validateQuery(query); + validateQuery(query, isMaster, false); if (count) { if (!classExists) { return 0; @@ -1809,7 +1806,7 @@ class DatabaseController { return Promise.resolve(response); } - static _validateQuery: any => void; + static _validateQuery: (any, boolean, boolean) => void; static filterSensitiveData: (boolean, any[], any, any, any, string, any[], any) => void; } diff --git a/src/RestQuery.js b/src/RestQuery.js index be96683451..a7a4a83f54 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -202,6 +202,9 @@ RestQuery.prototype.execute = function (executeOptions) { .then(() => { return this.buildRestWhere(); }) + .then(() => { + return this.denyProtectedFields(); + }) .then(() => { return this.handleIncludeAll(); }) @@ -688,6 +691,30 @@ RestQuery.prototype.runCount = function () { }); }; +RestQuery.prototype.denyProtectedFields = async function () { + if (this.auth.isMaster) { + return; + } + const schemaController = await this.config.database.loadSchema(); + const protectedFields = + this.config.database.addProtectedFields( + schemaController, + this.className, + this.restWhere, + this.findOptions.acl, + this.auth, + this.findOptions + ) || []; + for (const key of protectedFields) { + if (this.restWhere[key]) { + throw new Parse.Error( + Parse.Error.OPERATION_FORBIDDEN, + `This user is not allowed to query ${key} on class ${this.className}` + ); + } + } +}; + // Augments this.response with all pointers on an object RestQuery.prototype.handleIncludeAll = function () { if (!this.includeAll) {