Skip to content

Commit be4c7e2

Browse files
mtrezzamtrezza
authored
fix: Parse Pointer allows to access internal Parse Server classes and circumvent beforeFind query trigger; fixes security vulnerability [GHSA-fcv6-fg5r-jm9q](GHSA-fcv6-fg5r-jm9q)
1 parent 3289181 commit be4c7e2

12 files changed

+414
-224
lines changed

‎spec/CloudCode.spec.js

+29
Original file line numberDiff line numberDiff line change
@@ -2381,6 +2381,35 @@ describe('beforeFind hooks', () => {
23812381
})
23822382
.then(() => done());
23832383
});
2384+
2385+
it('should run beforeFind on pointers and array of pointers from an object', async () => {
2386+
const obj1 = new Parse.Object('TestObject');
2387+
const obj2 = new Parse.Object('TestObject2');
2388+
const obj3 = new Parse.Object('TestObject');
2389+
obj2.set('aField', 'aFieldValue');
2390+
await obj2.save();
2391+
obj1.set('pointerField', obj2);
2392+
obj3.set('pointerFieldArray', [obj2]);
2393+
await obj1.save();
2394+
await obj3.save();
2395+
const spy = jasmine.createSpy('beforeFindSpy');
2396+
Parse.Cloud.beforeFind('TestObject2', spy);
2397+
const query = new Parse.Query('TestObject');
2398+
await query.get(obj1.id);
2399+
// Pointer not included in query so we don't expect beforeFind to be called
2400+
expect(spy).not.toHaveBeenCalled();
2401+
const query2 = new Parse.Query('TestObject');
2402+
query2.include('pointerField');
2403+
const res = await query2.get(obj1.id);
2404+
expect(res.get('pointerField').get('aField')).toBe('aFieldValue');
2405+
// Pointer included in query so we expect beforeFind to be called
2406+
expect(spy).toHaveBeenCalledTimes(1);
2407+
const query3 = new Parse.Query('TestObject');
2408+
query3.include('pointerFieldArray');
2409+
const res2 = await query3.get(obj3.id);
2410+
expect(res2.get('pointerFieldArray')[0].get('aField')).toBe('aFieldValue');
2411+
expect(spy).toHaveBeenCalledTimes(2);
2412+
});
23842413
});
23852414

23862415
describe('afterFind hooks', () => {

‎spec/ParseGraphQLServer.spec.js

-1
Original file line numberDiff line numberDiff line change
@@ -5275,7 +5275,6 @@ describe('ParseGraphQLServer', () => {
52755275

52765276
it('should only count', async () => {
52775277
await prepareData();
5278-
52795278
await parseGraphQLServer.parseGraphQLSchema.schemaCache.clear();
52805279

52815280
const where = {

‎spec/ParseRole.spec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ describe('Parse Role testing', () => {
142142
return Promise.all(promises);
143143
};
144144

145-
const restExecute = spyOn(RestQuery.prototype, 'execute').and.callThrough();
145+
const restExecute = spyOn(RestQuery._UnsafeRestQuery.prototype, 'execute').and.callThrough();
146146

147147
let user, auth, getAllRolesSpy;
148148
createTestUser()

‎spec/RestQuery.spec.js

+24-20
Original file line numberDiff line numberDiff line change
@@ -399,15 +399,16 @@ describe('RestQuery.each', () => {
399399
}
400400
const config = Config.get('test');
401401
await Parse.Object.saveAll(objects);
402-
const query = new RestQuery(
402+
const query = await RestQuery({
403+
method: RestQuery.Method.find,
403404
config,
404-
auth.master(config),
405-
'Object',
406-
{ value: { $gt: 2 } },
407-
{ limit: 2 }
408-
);
405+
auth: auth.master(config),
406+
className: 'Object',
407+
restWhere: { value: { $gt: 2 } },
408+
restOptions: { limit: 2 },
409+
});
409410
const spy = spyOn(query, 'execute').and.callThrough();
410-
const classSpy = spyOn(RestQuery.prototype, 'execute').and.callThrough();
411+
const classSpy = spyOn(RestQuery._UnsafeRestQuery.prototype, 'execute').and.callThrough();
411412
const results = [];
412413
await query.each(result => {
413414
expect(result.value).toBeGreaterThan(2);
@@ -438,34 +439,37 @@ describe('RestQuery.each', () => {
438439
* Two queries needed since objectId are sorted and we can't know which one
439440
* going to be the first and then skip by the $gt added by each
440441
*/
441-
const queryOne = new RestQuery(
442+
const queryOne = await RestQuery({
443+
method: RestQuery.Method.get,
442444
config,
443-
auth.master(config),
444-
'Letter',
445-
{
445+
auth: auth.master(config),
446+
className: 'Letter',
447+
restWhere: {
446448
numbers: {
447449
__type: 'Pointer',
448450
className: 'Number',
449451
objectId: object1.id,
450452
},
451453
},
452-
{ limit: 1 }
453-
);
454-
const queryTwo = new RestQuery(
454+
restOptions: { limit: 1 },
455+
});
456+
457+
const queryTwo = await RestQuery({
458+
method: RestQuery.Method.get,
455459
config,
456-
auth.master(config),
457-
'Letter',
458-
{
460+
auth: auth.master(config),
461+
className: 'Letter',
462+
restWhere: {
459463
numbers: {
460464
__type: 'Pointer',
461465
className: 'Number',
462466
objectId: object2.id,
463467
},
464468
},
465-
{ limit: 1 }
466-
);
469+
restOptions: { limit: 1 },
470+
});
467471

468-
const classSpy = spyOn(RestQuery.prototype, 'execute').and.callThrough();
472+
const classSpy = spyOn(RestQuery._UnsafeRestQuery.prototype, 'execute').and.callThrough();
469473
const resultsOne = [];
470474
const resultsTwo = [];
471475
await queryOne.each(result => {

‎spec/rest.spec.js

+32
Original file line numberDiff line numberDiff line change
@@ -660,6 +660,38 @@ describe('rest create', () => {
660660
});
661661
});
662662

663+
it('cannot get object in volatileClasses if not masterKey through pointer', async () => {
664+
const masterKeyOnlyClassObject = new Parse.Object('_PushStatus');
665+
await masterKeyOnlyClassObject.save(null, { useMasterKey: true });
666+
const obj2 = new Parse.Object('TestObject');
667+
// Anyone is can basically create a pointer to any object
668+
// or some developers can use master key in some hook to link
669+
// private objects to standard objects
670+
obj2.set('pointer', masterKeyOnlyClassObject);
671+
await obj2.save();
672+
const query = new Parse.Query('TestObject');
673+
query.include('pointer');
674+
await expectAsync(query.get(obj2.id)).toBeRejectedWithError(
675+
"Clients aren't allowed to perform the get operation on the _PushStatus collection."
676+
);
677+
});
678+
679+
it('cannot get object in _GlobalConfig if not masterKey through pointer', async () => {
680+
await Parse.Config.save({ privateData: 'secret' }, { privateData: true });
681+
const obj2 = new Parse.Object('TestObject');
682+
obj2.set('globalConfigPointer', {
683+
__type: 'Pointer',
684+
className: '_GlobalConfig',
685+
objectId: 1,
686+
});
687+
await obj2.save();
688+
const query = new Parse.Query('TestObject');
689+
query.include('globalConfigPointer');
690+
await expectAsync(query.get(obj2.id)).toBeRejectedWithError(
691+
"Clients aren't allowed to perform the get operation on the _GlobalConfig collection."
692+
);
693+
});
694+
663695
it('locks down session', done => {
664696
let currentUser;
665697
Parse.User.#('foo', 'bar')

‎src/Auth.js

+37-9
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,15 @@ const getAuthForSessionToken = async function ({
9797
include: 'user',
9898
};
9999
const RestQuery = require('./RestQuery');
100-
const query = new RestQuery(config, master(config), '_Session', { sessionToken }, restOptions);
100+
const query = await RestQuery({
101+
method: RestQuery.Method.get,
102+
config,
103+
runBeforeFind: false,
104+
auth: master(config),
105+
className: '_Session',
106+
restWhere: { sessionToken },
107+
restOptions,
108+
});
101109
results = (await query.execute()).results;
102110
} else {
103111
results = (
@@ -134,12 +142,20 @@ const getAuthForSessionToken = async function ({
134142
});
135143
};
136144

137-
var getAuthForLegacySessionToken = function ({ config, sessionToken, installationId }) {
145+
var getAuthForLegacySessionToken = async function ({ config, sessionToken, installationId }) {
138146
var restOptions = {
139147
limit: 1,
140148
};
141149
const RestQuery = require('./RestQuery');
142-
var query = new RestQuery(config, master(config), '_User', { sessionToken }, restOptions);
150+
var query = await RestQuery({
151+
method: RestQuery.Method.get,
152+
config,
153+
runBeforeFind: false,
154+
auth: master(config),
155+
className: '_User',
156+
restWhere: { _session_token: sessionToken },
157+
restOptions,
158+
});
143159
return query.execute().then(response => {
144160
var results = response.results;
145161
if (results.length !== 1) {
@@ -184,9 +200,15 @@ Auth.prototype.getRolesForUser = async function () {
184200
},
185201
};
186202
const RestQuery = require('./RestQuery');
187-
await new RestQuery(this.config, master(this.config), '_Role', restWhere, {}).each(result =>
188-
results.push(result)
189-
);
203+
const query = await RestQuery({
204+
method: RestQuery.Method.find,
205+
runBeforeFind: false,
206+
config: this.config,
207+
auth: master(this.config),
208+
className: '_Role',
209+
restWhere,
210+
});
211+
await query.each(result => results.push(result));
190212
} else {
191213
await new Parse.Query(Parse.Role)
192214
.equalTo('users', this.user)
@@ -278,9 +300,15 @@ Auth.prototype.getRolesByIds = async function (ins) {
278300
});
279301
const restWhere = { roles: { $in: roles } };
280302
const RestQuery = require('./RestQuery');
281-
await new RestQuery(this.config, master(this.config), '_Role', restWhere, {}).each(result =>
282-
results.push(result)
283-
);
303+
const query = await RestQuery({
304+
method: RestQuery.Method.find,
305+
config: this.config,
306+
runBeforeFind: false,
307+
auth: master(this.config),
308+
className: '_Role',
309+
restWhere,
310+
});
311+
await query.each(result => results.push(result));
284312
}
285313
return results;
286314
};

‎src/Controllers/PushController.js

+9-2
Original file line numberDiff line numberDiff line change
@@ -58,9 +58,16 @@ export class PushController {
5858

5959
// Force filtering on only valid device tokens
6060
const updateWhere = applyDeviceTokenExists(where);
61-
badgeUpdate = () => {
61+
badgeUpdate = async () => {
6262
// Build a real RestQuery so we can use it in RestWrite
63-
const restQuery = new RestQuery(config, master(config), '_Installation', updateWhere);
63+
const restQuery = await RestQuery({
64+
method: RestQuery.Method.find,
65+
config,
66+
runBeforeFind: false,
67+
auth: master(config),
68+
className: '_Installation',
69+
restWhere: updateWhere,
70+
});
6471
return restQuery.buildRestWhere().then(() => {
6572
const write = new RestWrite(
6673
config,

‎src/Controllers/UserController.js

+18-5
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class UserController extends AdaptableController {
4848
}
4949
}
5050

51-
verifyEmail(username, token) {
51+
async verifyEmail(username, token) {
5252
if (!this.shouldVerifyEmails) {
5353
// Trying to verify email when not enabled
5454
// TODO: Better error here.
@@ -70,8 +70,14 @@ export class UserController extends AdaptableController {
7070
updateFields._email_verify_token_expires_at = { __op: 'Delete' };
7171
}
7272
const maintenanceAuth = Auth.maintenance(this.config);
73-
var findUserForEmailVerification = new RestQuery(this.config, maintenanceAuth, '_User', {
74-
username,
73+
var findUserForEmailVerification = await RestQuery({
74+
method: RestQuery.Method.get,
75+
config: this.config,
76+
auth: maintenanceAuth,
77+
className: '_User',
78+
restWhere: {
79+
username,
80+
},
7581
});
7682
return findUserForEmailVerification.execute().then(result => {
7783
if (result.results.length && result.results[0].emailVerified) {
@@ -110,7 +116,7 @@ export class UserController extends AdaptableController {
110116
});
111117
}
112118

113-
getUserIfNeeded(user) {
119+
async getUserIfNeeded(user) {
114120
if (user.username && user.email) {
115121
return Promise.resolve(user);
116122
}
@@ -122,7 +128,14 @@ export class UserController extends AdaptableController {
122128
where.email = user.email;
123129
}
124130

125-
var query = new RestQuery(this.config, Auth.master(this.config), '_User', where);
131+
var query = await RestQuery({
132+
method: RestQuery.Method.get,
133+
config: this.config,
134+
runBeforeFind: false,
135+
auth: Auth.master(this.config),
136+
className: '_User',
137+
restWhere: where,
138+
});
126139
return query.execute().then(function (result) {
127140
if (result.results.length != 1) {
128141
throw undefined;

0 commit comments

Comments
 (0)