Skip to content

Commit

Permalink
fix: brute force guessing of user sensitive data via search patterns (G…
Browse files Browse the repository at this point in the history
  • Loading branch information
mtrezza committed Sep 2, 2022
1 parent 5432082 commit 4c0c7c7
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 37 deletions.
73 changes: 73 additions & 0 deletions spec/RestQuery.spec.js
Expand Up @@ -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' })
Expand Down
71 changes: 34 additions & 37 deletions src/Controllers/DatabaseController.js
Expand Up @@ -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,
Expand All @@ -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}`);
}
});
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -761,7 +758,7 @@ class DatabaseController {
if (acl) {
query = addWriteACL(query, acl);
}
validateQuery(query);
validateQuery(query, isMaster, false);
return schemaController
.getOneSchema(className)
.catch(error => {
Expand Down Expand Up @@ -1253,7 +1250,7 @@ class DatabaseController {
query = addReadACL(query, aclGroup);
}
}
validateQuery(query);
validateQuery(query, isMaster, false);
if (count) {
if (!classExists) {
return 0;
Expand Down Expand Up @@ -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;
}

Expand Down
27 changes: 27 additions & 0 deletions src/RestQuery.js
Expand Up @@ -202,6 +202,9 @@ RestQuery.prototype.execute = function (executeOptions) {
.then(() => {
return this.buildRestWhere();
})
.then(() => {
return this.denyProtectedFields();
})
.then(() => {
return this.handleIncludeAll();
})
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 4c0c7c7

Please sign in to comment.