Skip to content

Commit 1de4e43

Browse files
mtrezzamtrezza
authored
fix: Classes _GraphQLConfig and _Audience master key bypass via generic class routes ([GHSA-7xg7-rqf6-pw6c](GHSA-7xg7-rqf6-pw6c)) (#10151)
1 parent 4625aef commit 1de4e43

File tree

4 files changed

+162
-12
lines changed

4 files changed

+162
-12
lines changed

‎spec/AudienceRouter.spec.js‎

Lines changed: 12 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ describe('AudiencesRouter', () => {
3636

3737
const router = new AudiencesRouter();
3838
rest
39-
.create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
39+
.create(config, auth.master(config), '_Audience', androidAudienceRequest)
4040
.then(() => {
41-
return rest.create(config, auth.nobody(config), '_Audience', iosAudienceRequest);
41+
return rest.create(config, auth.master(config), '_Audience', iosAudienceRequest);
4242
})
4343
.then(() => {
4444
return router.handleFind(request);
@@ -78,9 +78,9 @@ describe('AudiencesRouter', () => {
7878

7979
const router = new AudiencesRouter();
8080
rest
81-
.create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
81+
.create(config, auth.master(config), '_Audience', androidAudienceRequest)
8282
.then(() => {
83-
return rest.create(config, auth.nobody(config), '_Audience', iosAudienceRequest);
83+
return rest.create(config, auth.master(config), '_Audience', iosAudienceRequest);
8484
})
8585
.then(() => {
8686
return router.handleFind(request);
@@ -119,9 +119,9 @@ describe('AudiencesRouter', () => {
119119
Config.get('test');
120120
const router = new AudiencesRouter();
121121
rest
122-
.create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
122+
.create(config, auth.master(config), '_Audience', androidAudienceRequest)
123123
.then(() => {
124-
return rest.create(config, auth.nobody(config), '_Audience', iosAudienceRequest);
124+
return rest.create(config, auth.master(config), '_Audience', iosAudienceRequest);
125125
})
126126
.then(() => {
127127
return router.handleFind(request);
@@ -159,8 +159,8 @@ describe('AudiencesRouter', () => {
159159

160160
const router = new AudiencesRouter();
161161
rest
162-
.create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
163-
.then(() => rest.create(config, auth.nobody(config), '_Audience', iosAudienceRequest))
162+
.create(config, auth.master(config), '_Audience', androidAudienceRequest)
163+
.then(() => rest.create(config, auth.master(config), '_Audience', iosAudienceRequest))
164164
.then(() => router.handleFind(request))
165165
.then(res => {
166166
const response = res.response;
@@ -197,9 +197,9 @@ describe('AudiencesRouter', () => {
197197

198198
const router = new AudiencesRouter();
199199
rest
200-
.create(config, auth.nobody(config), '_Audience', androidAudienceRequest)
200+
.create(config, auth.master(config), '_Audience', androidAudienceRequest)
201201
.then(() => {
202-
return rest.create(config, auth.nobody(config), '_Audience', iosAudienceRequest);
202+
return rest.create(config, auth.master(config), '_Audience', iosAudienceRequest);
203203
})
204204
.then(() => {
205205
return router.handleFind(request);
@@ -421,6 +421,7 @@ describe('AudiencesRouter', () => {
421421
await reconfigureServer({
422422
appId: 'test',
423423
restAPIKey: 'test',
424+
masterKey: 'test',
424425
publicServerURL: 'http://localhost:8378/1',
425426
});
426427
try {
@@ -430,7 +431,7 @@ describe('AudiencesRouter', () => {
430431
body: { lorem: 'ipsum', _method: 'POST' },
431432
headers: {
432433
'X-Parse-Application-Id': 'test',
433-
'X-Parse-REST-API-Key': 'test',
434+
'X-Parse-Master-Key': 'test',
434435
'Content-Type': 'application/json',
435436
},
436437
});

‎spec/PushController.spec.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1074,7 +1074,7 @@ describe('PushController', () => {
10741074
const audience = new Parse.Object('_Audience');
10751075
audience.set('name', 'testAudience');
10761076
audience.set('query', JSON.stringify(where));
1077-
await Parse.Object.saveAll(audience);
1077+
await audience.save(null, { useMasterKey: true });
10781078
await query.find({ useMasterKey: true }).then(parseResults);
10791079

10801080
const body = {

‎spec/rest.spec.js‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -812,6 +812,153 @@ describe('rest create', () => {
812812
expect(loggerErrorSpy).toHaveBeenCalledWith('Sanitized error:', jasmine.stringContaining("Clients aren't allowed to perform the get operation on the _GlobalConfig collection."));
813813
});
814814

815+
it('should require master key for all volatile classes', () => {
816+
// This test guards against drift between volatileClasses (SchemaController.js)
817+
// and classesWithMasterOnlyAccess (SharedRest.js). If a new volatile class is
818+
// added, it must also be added to classesWithMasterOnlyAccess and this test.
819+
const volatileClasses = [
820+
'_JobStatus',
821+
'_PushStatus',
822+
'_Hooks',
823+
'_GlobalConfig',
824+
'_GraphQLConfig',
825+
'_JobSchedule',
826+
'_Audience',
827+
'_Idempotency',
828+
];
829+
for (const className of volatileClasses) {
830+
expect(() =>
831+
rest.create(config, auth.nobody(config), className, {})
832+
).toThrowMatching(
833+
e => e.code === Parse.Error.OPERATION_FORBIDDEN,
834+
`Expected ${className} to require master key`
835+
);
836+
}
837+
});
838+
839+
it('cannot find objects in _GraphQLConfig without masterKey', async () => {
840+
await config.parseGraphQLController.updateGraphQLConfig({ enabledForClasses: ['_User'] });
841+
await expectAsync(
842+
rest.find(config, auth.nobody(config), '_GraphQLConfig', {})
843+
).toBeRejectedWith(
844+
jasmine.objectContaining({ code: Parse.Error.OPERATION_FORBIDDEN })
845+
);
846+
});
847+
848+
it('cannot update object in _GraphQLConfig without masterKey', async () => {
849+
await config.parseGraphQLController.updateGraphQLConfig({ enabledForClasses: ['_User'] });
850+
expect(() =>
851+
rest.update(config, auth.nobody(config), '_GraphQLConfig', '1', {
852+
config: { enabledForClasses: [] },
853+
})
854+
).toThrowMatching(e => e.code === Parse.Error.OPERATION_FORBIDDEN);
855+
});
856+
857+
it('cannot delete object in _GraphQLConfig without masterKey', async () => {
858+
await config.parseGraphQLController.updateGraphQLConfig({ enabledForClasses: ['_User'] });
859+
expect(() =>
860+
rest.del(config, auth.nobody(config), '_GraphQLConfig', '1')
861+
).toThrowMatching(e => e.code === Parse.Error.OPERATION_FORBIDDEN);
862+
});
863+
864+
it('can perform operations on _GraphQLConfig with masterKey', async () => {
865+
await config.parseGraphQLController.updateGraphQLConfig({ enabledForClasses: ['_User'] });
866+
const found = await rest.find(config, auth.master(config), '_GraphQLConfig', {});
867+
expect(found.results.length).toBeGreaterThan(0);
868+
await rest.del(config, auth.master(config), '_GraphQLConfig', '1');
869+
const afterDelete = await rest.find(config, auth.master(config), '_GraphQLConfig', {});
870+
expect(afterDelete.results.length).toBe(0);
871+
});
872+
873+
it('cannot create object in _Audience without masterKey', () => {
874+
expect(() =>
875+
rest.create(config, auth.nobody(config), '_Audience', {
876+
name: 'test',
877+
query: '{}',
878+
})
879+
).toThrowMatching(e => e.code === Parse.Error.OPERATION_FORBIDDEN);
880+
});
881+
882+
it('cannot find objects in _Audience without masterKey', async () => {
883+
await expectAsync(
884+
rest.find(config, auth.nobody(config), '_Audience', {})
885+
).toBeRejectedWith(
886+
jasmine.objectContaining({ code: Parse.Error.OPERATION_FORBIDDEN })
887+
);
888+
});
889+
890+
it('cannot update object in _Audience without masterKey', async () => {
891+
const obj = await rest.create(config, auth.master(config), '_Audience', {
892+
name: 'test',
893+
query: '{}',
894+
});
895+
expect(() =>
896+
rest.update(config, auth.nobody(config), '_Audience', obj.response.objectId, {
897+
name: 'updated',
898+
})
899+
).toThrowMatching(e => e.code === Parse.Error.OPERATION_FORBIDDEN);
900+
});
901+
902+
it('cannot delete object in _Audience without masterKey', async () => {
903+
const obj = await rest.create(config, auth.master(config), '_Audience', {
904+
name: 'test',
905+
query: '{}',
906+
});
907+
expect(() =>
908+
rest.del(config, auth.nobody(config), '_Audience', obj.response.objectId)
909+
).toThrowMatching(e => e.code === Parse.Error.OPERATION_FORBIDDEN);
910+
});
911+
912+
it('can perform CRUD on _Audience with masterKey', async () => {
913+
const obj = await rest.create(config, auth.master(config), '_Audience', {
914+
name: 'test',
915+
query: '{}',
916+
});
917+
expect(obj.response.objectId).toBeDefined();
918+
const found = await rest.find(config, auth.master(config), '_Audience', {});
919+
expect(found.results.length).toBeGreaterThan(0);
920+
await rest.del(config, auth.master(config), '_Audience', obj.response.objectId);
921+
const afterDelete = await rest.find(config, auth.master(config), '_Audience', {});
922+
expect(afterDelete.results.length).toBe(0);
923+
});
924+
925+
it('cannot access _GraphQLConfig via class route without masterKey', async () => {
926+
await config.parseGraphQLController.updateGraphQLConfig({ enabledForClasses: ['_User'] });
927+
try {
928+
await request({
929+
url: 'http://localhost:8378/1/classes/_GraphQLConfig',
930+
json: true,
931+
headers: {
932+
'X-Parse-Application-Id': 'test',
933+
'X-Parse-REST-API-Key': 'rest',
934+
},
935+
});
936+
fail('should have thrown');
937+
} catch (e) {
938+
expect(e.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
939+
}
940+
});
941+
942+
it('cannot access _Audience via class route without masterKey', async () => {
943+
await rest.create(config, auth.master(config), '_Audience', {
944+
name: 'test',
945+
query: '{}',
946+
});
947+
try {
948+
await request({
949+
url: 'http://localhost:8378/1/classes/_Audience',
950+
json: true,
951+
headers: {
952+
'X-Parse-Application-Id': 'test',
953+
'X-Parse-REST-API-Key': 'rest',
954+
},
955+
});
956+
fail('should have thrown');
957+
} catch (e) {
958+
expect(e.data.code).toBe(Parse.Error.OPERATION_FORBIDDEN);
959+
}
960+
});
961+
815962
it('locks down session', done => {
816963
let currentUser;
817964
Parse.User.signUp('foo', 'bar')

‎src/SharedRest.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@ const classesWithMasterOnlyAccess = [
33
'_PushStatus',
44
'_Hooks',
55
'_GlobalConfig',
6+
'_GraphQLConfig',
67
'_JobSchedule',
8+
'_Audience',
79
'_Idempotency',
810
];
911
const { createSanitizedError } = require('./Error');

0 commit comments

Comments
 (0)
⚡