Skip to content

Commit 6458ab0

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 62bb396 commit 6458ab0

12 files changed

+412
-224
lines changed

‎spec/CloudCode.spec.js

+29
Original file line numberDiff line numberDiff line change
@@ -2342,6 +2342,35 @@ describe('beforeFind hooks', () => {
23422342
})
23432343
.then(() => done());
23442344
});
2345+
2346+
it('should run beforeFind on pointers and array of pointers from an object', async () => {
2347+
const obj1 = new Parse.Object('TestObject');
2348+
const obj2 = new Parse.Object('TestObject2');
2349+
const obj3 = new Parse.Object('TestObject');
2350+
obj2.set('aField', 'aFieldValue');
2351+
await obj2.save();
2352+
obj1.set('pointerField', obj2);
2353+
obj3.set('pointerFieldArray', [obj2]);
2354+
await obj1.save();
2355+
await obj3.save();
2356+
const spy = jasmine.createSpy('beforeFindSpy');
2357+
Parse.Cloud.beforeFind('TestObject2', spy);
2358+
const query = new Parse.Query('TestObject');
2359+
await query.get(obj1.id);
2360+
// Pointer not included in query so we don't expect beforeFind to be called
2361+
expect(spy).not.toHaveBeenCalled();
2362+
const query2 = new Parse.Query('TestObject');
2363+
query2.include('pointerField');
2364+
const res = await query2.get(obj1.id);
2365+
expect(res.get('pointerField').get('aField')).toBe('aFieldValue');
2366+
// Pointer included in query so we expect beforeFind to be called
2367+
expect(spy).toHaveBeenCalledTimes(1);
2368+
const query3 = new Parse.Query('TestObject');
2369+
query3.include('pointerFieldArray');
2370+
const res2 = await query3.get(obj3.id);
2371+
expect(res2.get('pointerFieldArray')[0].get('aField')).toBe('aFieldValue');
2372+
expect(spy).toHaveBeenCalledTimes(2);
2373+
});
23452374
});
23462375

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

‎spec/ParseGraphQLServer.spec.js

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

52705270
it('should only count', async () => {
52715271
await prepareData();
5272-
52735272
await parseGraphQLServer.parseGraphQLSchema.schemaCache.clear();
52745273

52755274
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
@@ -398,15 +398,16 @@ describe('RestQuery.each', () => {
398398
}
399399
const config = Config.get('test');
400400
await Parse.Object.saveAll(objects);
401-
const query = new RestQuery(
401+
const query = await RestQuery({
402+
method: RestQuery.Method.find,
402403
config,
403-
auth.master(config),
404-
'Object',
405-
{ value: { $gt: 2 } },
406-
{ limit: 2 }
407-
);
404+
auth: auth.master(config),
405+
className: 'Object',
406+
restWhere: { value: { $gt: 2 } },
407+
restOptions: { limit: 2 },
408+
});
408409
const spy = spyOn(query, 'execute').and.callThrough();
409-
const classSpy = spyOn(RestQuery.prototype, 'execute').and.callThrough();
410+
const classSpy = spyOn(RestQuery._UnsafeRestQuery.prototype, 'execute').and.callThrough();
410411
const results = [];
411412
await query.each(result => {
412413
expect(result.value).toBeGreaterThan(2);
@@ -437,34 +438,37 @@ describe('RestQuery.each', () => {
437438
* Two queries needed since objectId are sorted and we can't know which one
438439
* going to be the first and then skip by the $gt added by each
439440
*/
440-
const queryOne = new RestQuery(
441+
const queryOne = await RestQuery({
442+
method: RestQuery.Method.get,
441443
config,
442-
auth.master(config),
443-
'Letter',
444-
{
444+
auth: auth.master(config),
445+
className: 'Letter',
446+
restWhere: {
445447
numbers: {
446448
__type: 'Pointer',
447449
className: 'Number',
448450
objectId: object1.id,
449451
},
450452
},
451-
{ limit: 1 }
452-
);
453-
const queryTwo = new RestQuery(
453+
restOptions: { limit: 1 },
454+
});
455+
456+
const queryTwo = await RestQuery({
457+
method: RestQuery.Method.get,
454458
config,
455-
auth.master(config),
456-
'Letter',
457-
{
459+
auth: auth.master(config),
460+
className: 'Letter',
461+
restWhere: {
458462
numbers: {
459463
__type: 'Pointer',
460464
className: 'Number',
461465
objectId: object2.id,
462466
},
463467
},
464-
{ limit: 1 }
465-
);
468+
restOptions: { limit: 1 },
469+
});
466470

467-
const classSpy = spyOn(RestQuery.prototype, 'execute').and.callThrough();
471+
const classSpy = spyOn(RestQuery._UnsafeRestQuery.prototype, 'execute').and.callThrough();
468472
const resultsOne = [];
469473
const resultsTwo = [];
470474
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.signUp('foo', 'bar')

‎src/Auth.js

+39-9
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,15 @@ const getAuthForSessionToken = async function ({
8484
include: 'user',
8585
};
8686

87-
const query = new RestQuery(config, master(config), '_Session', { sessionToken }, restOptions);
87+
const query = await RestQuery({
88+
method: RestQuery.Method.get,
89+
runBeforeFind: false,
90+
config,
91+
auth: master(config),
92+
className: '_Session',
93+
restWhere: { sessionToken },
94+
restOptions,
95+
});
8896
results = (await query.execute()).results;
8997
} else {
9098
results = (
@@ -121,11 +129,19 @@ const getAuthForSessionToken = async function ({
121129
});
122130
};
123131

124-
var getAuthForLegacySessionToken = function ({ config, sessionToken, installationId }) {
132+
var getAuthForLegacySessionToken = async function ({ config, sessionToken, installationId }) {
125133
var restOptions = {
126134
limit: 1,
127135
};
128-
var query = new RestQuery(config, master(config), '_User', { sessionToken }, restOptions);
136+
var query = await RestQuery({
137+
method: RestQuery.Method.get,
138+
runBeforeFind: false,
139+
config,
140+
auth: master(config),
141+
className: '_User',
142+
restWhere: { sessionToken },
143+
restOptions,
144+
});
129145
return query.execute().then(response => {
130146
var results = response.results;
131147
if (results.length !== 1) {
@@ -169,9 +185,16 @@ Auth.prototype.getRolesForUser = async function () {
169185
objectId: this.user.id,
170186
},
171187
};
172-
await new RestQuery(this.config, master(this.config), '_Role', restWhere, {}).each(result =>
173-
results.push(result)
174-
);
188+
const query = await RestQuery({
189+
method: RestQuery.Method.find,
190+
config: this.config,
191+
auth: master(this.config),
192+
runBeforeFind: false,
193+
className: '_Role',
194+
restWhere,
195+
restOptions: {},
196+
});
197+
await query.each(result => results.push(result));
175198
} else {
176199
await new Parse.Query(Parse.Role)
177200
.equalTo('users', this.user)
@@ -262,9 +285,16 @@ Auth.prototype.getRolesByIds = async function (ins) {
262285
};
263286
});
264287
const restWhere = { roles: { $in: roles } };
265-
await new RestQuery(this.config, master(this.config), '_Role', restWhere, {}).each(result =>
266-
results.push(result)
267-
);
288+
const query = await RestQuery({
289+
method: RestQuery.Method.find,
290+
runBeforeFind: false,
291+
config: this.config,
292+
auth: master(this.config),
293+
className: '_Role',
294+
restWhere,
295+
restOptions: {},
296+
});
297+
await query.each(result => results.push(result));
268298
}
269299
return results;
270300
};

‎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-9
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,12 +70,14 @@ export class UserController extends AdaptableController {
7070
updateFields._email_verify_token_expires_at = { __op: 'Delete' };
7171
}
7272
const masterAuth = Auth.master(this.config);
73-
var findUserForEmailVerification = new RestQuery(
74-
this.config,
75-
Auth.master(this.config),
76-
'_User',
77-
{ username: username }
78-
);
73+
var findUserForEmailVerification = await RestQuery({
74+
method: RestQuery.Method.get,
75+
config: this.config,
76+
runBeforeFind: false,
77+
auth: Auth.master(this.config),
78+
className: '_User',
79+
restWhere: { username },
80+
});
7981
return findUserForEmailVerification.execute().then(result => {
8082
if (result.results.length && result.results[0].emailVerified) {
8183
return Promise.resolve(result.results.length[0]);
@@ -112,7 +114,7 @@ export class UserController extends AdaptableController {
112114
});
113115
}
114116

115-
getUserIfNeeded(user) {
117+
async getUserIfNeeded(user) {
116118
if (user.username && user.email) {
117119
return Promise.resolve(user);
118120
}
@@ -124,7 +126,14 @@ export class UserController extends AdaptableController {
124126
where.email = user.email;
125127
}
126128

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

0 commit comments

Comments
 (0)