fix: brute force guessing of user sensitive data via search patterns (G…

…HSA-2m6g-crv8-p3c6) (parse-community#8146) [skip release]
dblythy · Sep 2, 2022 · 4c0c7c7 · 4c0c7c7
1 parent 5432082
commit 4c0c7c7
Show file tree

Hide file tree

Showing 3 changed files with 134 additions and 37 deletions.
diff --git 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
@@ -55,47 +55,43 @@ 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.');
     }
   }
 
   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.');
     }
   }
 
   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
@@ -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) {