Skip to content

Commit 2a60732

Browse files
authored
fix: Unauthenticated deletion of installation records via operator injection in device token deduplication ([GHSA-cc6h-c8m4-hgrx](GHSA-cc6h-c8m4-hgrx)) (#10658)
1 parent 609f615 commit 2a60732

2 files changed

Lines changed: 367 additions & 1 deletion

File tree

spec/vulnerabilities.spec.js

Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6088,4 +6088,325 @@ describe('Vulnerabilities', () => {
60886088
await sleep(0);
60896089
});
60906090
});
6091+
6092+
describe('(GHSA-cc6h-c8m4-hgrx) NoSQL injection via _Installation deviceToken deduplication', () => {
6093+
const serverURL = 'http://localhost:8378/1';
6094+
const publicHeaders = {
6095+
'X-Parse-Application-Id': 'test',
6096+
'X-Parse-REST-API-Key': 'rest',
6097+
'Content-Type': 'application/json',
6098+
};
6099+
const attackerInstallationId = 'attacker-uuid-0000-0000-000000000000';
6100+
const { sleep } = require('../lib/TestUtils');
6101+
6102+
const postInstallation = body =>
6103+
request({
6104+
method: 'POST',
6105+
headers: publicHeaders,
6106+
url: `${serverURL}/installations`,
6107+
body: JSON.stringify(body),
6108+
}).catch(e => e);
6109+
6110+
const putInstallation = (objectId, body) =>
6111+
request({
6112+
method: 'PUT',
6113+
headers: publicHeaders,
6114+
url: `${serverURL}/installations/${objectId}`,
6115+
body: JSON.stringify(body),
6116+
}).catch(e => e);
6117+
6118+
const allInstallations = async () => {
6119+
// `handleInstallation` does not await its deduplication delete, so give any pending
6120+
// delete time to land before asserting; otherwise an assertion that rows survived
6121+
// could pass simply because the delete had not run yet.
6122+
await sleep(100);
6123+
const query = new Parse.Query(Parse.Installation);
6124+
query.limit(1000);
6125+
const results = await query.find({ useMasterKey: true });
6126+
return results.map(r => r.get('installationId')).sort();
6127+
};
6128+
6129+
// Registers `count` unrelated installations, each with its own installationId and
6130+
// deviceToken, exactly as a device SDK would.
6131+
const seedVictimInstallations = async count => {
6132+
for (let i = 0; i < count; i++) {
6133+
const response = await postInstallation({
6134+
installationId: `victim-uuid-0000-0000-00000000000${i}`,
6135+
deviceType: 'ios',
6136+
deviceToken: `victimtoken${i}`,
6137+
});
6138+
expect(response.status).toBe(201);
6139+
}
6140+
};
6141+
6142+
// Doubles as a positive control: proves the unauthenticated client really reaches the
6143+
// application, so a later "nothing was deleted" result cannot be a broken harness.
6144+
const registerAttackerInstallation = async () => {
6145+
const response = await postInstallation({
6146+
installationId: attackerInstallationId,
6147+
deviceType: 'android',
6148+
});
6149+
expect(response.status).toBe(201);
6150+
};
6151+
6152+
it('does not delete other installations when deviceToken is an operator object', async () => {
6153+
await seedVictimInstallations(4);
6154+
await registerAttackerInstallation();
6155+
expect((await allInstallations()).length).toBe(5);
6156+
6157+
const response = await postInstallation({
6158+
installationId: attackerInstallationId,
6159+
deviceToken: { $ne: null },
6160+
});
6161+
6162+
expect(response.status).toBe(400);
6163+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6164+
expect((await allInstallations()).length).toBe(5);
6165+
});
6166+
6167+
it('does not delete other installations when deviceToken is an operator object and no installation matches the installationId', async () => {
6168+
await seedVictimInstallations(4);
6169+
expect((await allInstallations()).length).toBe(4);
6170+
6171+
// No prior registration, so the request reaches the deduplication branch that runs
6172+
// when no row matches the installationId.
6173+
const response = await postInstallation({
6174+
installationId: 'unregistered-uuid-0000-0000-0000',
6175+
deviceType: 'android',
6176+
deviceToken: { $ne: null },
6177+
});
6178+
6179+
expect(response.status).toBe(400);
6180+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6181+
expect((await allInstallations()).length).toBe(4);
6182+
});
6183+
6184+
it('does not delete targeted installations when deviceToken is a regex operator', async () => {
6185+
await seedVictimInstallations(4);
6186+
await registerAttackerInstallation();
6187+
6188+
const response = await postInstallation({
6189+
installationId: attackerInstallationId,
6190+
deviceToken: { $regex: '^victimtoken' },
6191+
});
6192+
6193+
expect(response.status).toBe(400);
6194+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6195+
expect((await allInstallations()).length).toBe(5);
6196+
});
6197+
6198+
it('does not delete other installations when deviceToken is an operator object on update', async () => {
6199+
await seedVictimInstallations(4);
6200+
const created = await postInstallation({
6201+
installationId: attackerInstallationId,
6202+
deviceType: 'android',
6203+
});
6204+
expect(created.status).toBe(201);
6205+
6206+
const response = await putInstallation(created.data.objectId, {
6207+
deviceToken: { $ne: null },
6208+
});
6209+
6210+
expect(response.status).toBe(400);
6211+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6212+
expect((await allInstallations()).length).toBe(5);
6213+
});
6214+
6215+
it('does not delete other installations when appIdentifier is an operator object', async () => {
6216+
await seedVictimInstallations(4);
6217+
await registerAttackerInstallation();
6218+
6219+
const response = await postInstallation({
6220+
installationId: attackerInstallationId,
6221+
deviceToken: 'victimtoken0',
6222+
appIdentifier: { $ne: null },
6223+
});
6224+
6225+
expect(response.status).toBe(400);
6226+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6227+
expect((await allInstallations()).length).toBe(5);
6228+
});
6229+
6230+
it('rejects a non-string installationId with a client error', async () => {
6231+
await seedVictimInstallations(1);
6232+
6233+
const response = await postInstallation({
6234+
installationId: { $ne: null },
6235+
deviceType: 'android',
6236+
deviceToken: 'sometoken',
6237+
});
6238+
6239+
expect(response.status).toBe(400);
6240+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6241+
expect((await allInstallations()).length).toBe(1);
6242+
});
6243+
6244+
it('still allows appIdentifier to be unset with a Delete operation', async () => {
6245+
const created = await postInstallation({
6246+
installationId: 'device-uuid-0000-0000-000000000009',
6247+
deviceType: 'ios',
6248+
deviceToken: 'unsettoken',
6249+
appIdentifier: 'com.example.app',
6250+
});
6251+
expect(created.status).toBe(201);
6252+
6253+
// `appIdentifier` only narrows the deduplication query, so unsetting it is a valid
6254+
// operation that must survive the type validation above.
6255+
const response = await putInstallation(created.data.objectId, {
6256+
appIdentifier: { __op: 'Delete' },
6257+
});
6258+
6259+
expect(response.status).toBe(200);
6260+
const query = new Parse.Query(Parse.Installation);
6261+
query.equalTo('objectId', created.data.objectId);
6262+
const [installation] = await query.find({ useMasterKey: true });
6263+
expect(installation.get('appIdentifier')).toBeUndefined();
6264+
});
6265+
6266+
it('does not clean up installations of other applications when appIdentifier is unset', async () => {
6267+
const victim = await postInstallation({
6268+
installationId: 'victim-uuid-0000-0000-00000000009',
6269+
deviceType: 'ios',
6270+
deviceToken: 'contested-token',
6271+
appIdentifier: 'com.example.victimapp',
6272+
});
6273+
expect(victim.status).toBe(201);
6274+
const attacker = await postInstallation({
6275+
installationId: attackerInstallationId,
6276+
deviceType: 'android',
6277+
deviceToken: 'attacker-token',
6278+
appIdentifier: 'com.example.attackerapp',
6279+
});
6280+
expect(attacker.status).toBe(201);
6281+
6282+
// Claiming the other application's device token while unsetting `appIdentifier` must
6283+
// not drop the constraint that scopes the cleanup to the caller's own application.
6284+
const response = await postInstallation({
6285+
installationId: attackerInstallationId,
6286+
deviceToken: 'contested-token',
6287+
appIdentifier: { __op: 'Delete' },
6288+
});
6289+
expect(response.status).toBe(200);
6290+
6291+
expect(await allInstallations()).toEqual(
6292+
['victim-uuid-0000-0000-00000000009', attackerInstallationId].sort()
6293+
);
6294+
});
6295+
6296+
it('reports the received type when a deviceToken is an array', async () => {
6297+
await seedVictimInstallations(1);
6298+
await registerAttackerInstallation();
6299+
6300+
const response = await postInstallation({
6301+
installationId: attackerInstallationId,
6302+
deviceToken: ['victimtoken0'],
6303+
});
6304+
6305+
expect(response.status).toBe(400);
6306+
expect(response.data.code).toBe(Parse.Error.INCORRECT_TYPE);
6307+
expect(response.data.error).toBe(
6308+
'schema mismatch for _Installation.deviceToken; expected String but got Array'
6309+
);
6310+
expect((await allInstallations()).length).toBe(2);
6311+
});
6312+
6313+
it('skips the cleanup when appIdentifier is unset and the matched installation has none', async () => {
6314+
const victim = await postInstallation({
6315+
installationId: 'victim-uuid-0000-0000-00000000010',
6316+
deviceType: 'ios',
6317+
deviceToken: 'unscoped-token',
6318+
appIdentifier: 'com.example.victimapp',
6319+
});
6320+
expect(victim.status).toBe(201);
6321+
// The caller's own installation carries no application scope to fall back to.
6322+
const attacker = await postInstallation({
6323+
installationId: attackerInstallationId,
6324+
deviceType: 'android',
6325+
deviceToken: 'attacker-token',
6326+
});
6327+
expect(attacker.status).toBe(201);
6328+
6329+
const response = await postInstallation({
6330+
installationId: attackerInstallationId,
6331+
deviceToken: 'unscoped-token',
6332+
appIdentifier: { __op: 'Delete' },
6333+
});
6334+
6335+
expect(response.status).toBe(200);
6336+
expect(await allInstallations()).toEqual(
6337+
['victim-uuid-0000-0000-00000000010', attackerInstallationId].sort()
6338+
);
6339+
});
6340+
6341+
it('skips the cleanup when appIdentifier is unset and no installation matches', async () => {
6342+
const first = await postInstallation({
6343+
installationId: 'victim-uuid-0000-0000-00000000011',
6344+
deviceType: 'ios',
6345+
deviceToken: 'collide-token',
6346+
appIdentifier: 'com.example.appone',
6347+
});
6348+
expect(first.status).toBe(201);
6349+
const second = await postInstallation({
6350+
installationId: 'victim-uuid-0000-0000-00000000012',
6351+
deviceType: 'ios',
6352+
deviceToken: 'collide-token-2',
6353+
appIdentifier: 'com.example.apptwo',
6354+
});
6355+
expect(second.status).toBe(201);
6356+
6357+
// An unregistered installationId reaches the branch that runs when nothing matches.
6358+
const response = await postInstallation({
6359+
installationId: 'unregistered-uuid-0000-0000-0001',
6360+
deviceType: 'android',
6361+
deviceToken: 'collide-token',
6362+
appIdentifier: { __op: 'Delete' },
6363+
});
6364+
6365+
expect(response.status).toBe(201);
6366+
expect(await allInstallations()).toEqual(
6367+
[
6368+
'victim-uuid-0000-0000-00000000011',
6369+
'victim-uuid-0000-0000-00000000012',
6370+
'unregistered-uuid-0000-0000-0001',
6371+
].sort()
6372+
);
6373+
});
6374+
6375+
it('guards every _Installation field that the schema declares as String and the deduplication queries use', () => {
6376+
// The guard in `handleInstallation` hardcodes `String` because the schema's own type
6377+
// check runs too late in the write pipeline to be reused. This pins the two together:
6378+
// if a field is renamed or redeclared, this fails rather than leaving a stale guard.
6379+
const { defaultColumns } = require('../lib/Controllers/SchemaController');
6380+
for (const fieldName of ['deviceToken', 'installationId', 'appIdentifier']) {
6381+
expect(defaultColumns._Installation[fieldName]).toEqual({ type: 'String' });
6382+
}
6383+
});
6384+
6385+
it('still deduplicates installations that share a string deviceToken', async () => {
6386+
const first = await postInstallation({
6387+
installationId: 'device-uuid-0000-0000-000000000001',
6388+
deviceType: 'ios',
6389+
deviceToken: 'sharedtoken',
6390+
});
6391+
expect(first.status).toBe(201);
6392+
6393+
// The same physical device re-registers under a new installationId: the stale row
6394+
// holding the device token must still be cleaned up.
6395+
const second = await postInstallation({
6396+
installationId: 'device-uuid-0000-0000-000000000002',
6397+
deviceType: 'ios',
6398+
deviceToken: 'sharedtoken',
6399+
});
6400+
expect(second.status).toBe(201);
6401+
6402+
// `handleInstallation` does not await the deduplication delete, so poll for it
6403+
// rather than assuming it has completed by the time the response is returned.
6404+
let installations = await allInstallations();
6405+
for (let attempt = 0; attempt < 20 && installations.length > 1; attempt++) {
6406+
await sleep(50);
6407+
installations = await allInstallations();
6408+
}
6409+
expect(installations).toEqual(['device-uuid-0000-0000-000000000002']);
6410+
});
6411+
});
60916412
});

src/RestWrite.js

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1231,6 +1231,33 @@ RestWrite.prototype.handleInstallation = function () {
12311231
return;
12321232
}
12331233

1234+
// The deduplication below embeds these client-supplied values directly into database
1235+
// queries that delete or update rows with master privileges, and it runs before
1236+
// `validateSchema`, so their types must be enforced here: a non-string value would
1237+
// otherwise reach the database as a query constraint (such as an operator object
1238+
// `{"$ne": null}`) matching rows the client never identified, instead of as a literal
1239+
// value to match against. The schema declares all three as `String`, but that check
1240+
// cannot be reused here; it runs later in the write pipeline and moving it earlier
1241+
// would mutate the schema before the permission check. The field list is a property of
1242+
// this function rather than of the schema: it is the set of values spliced into the
1243+
// deduplication queries below.
1244+
for (const fieldName of ['deviceToken', 'installationId', 'appIdentifier']) {
1245+
const value = this.data[fieldName];
1246+
if (value === undefined || value === null || typeof value === 'string') {
1247+
continue;
1248+
}
1249+
if (fieldName === 'appIdentifier' && value.__op === 'Delete') {
1250+
continue;
1251+
}
1252+
const actualType = Array.isArray(value)
1253+
? 'Array'
1254+
: `${typeof value}`.replace(/^./, character => character.toUpperCase());
1255+
throw new Parse.Error(
1256+
Parse.Error.INCORRECT_TYPE,
1257+
`schema mismatch for _Installation.${fieldName}; expected String but got ${actualType}`
1258+
);
1259+
}
1260+
12341261
if (
12351262
!this.query &&
12361263
!this.data.deviceToken &&
@@ -1393,6 +1420,12 @@ RestWrite.prototype.handleInstallation = function () {
13931420
},
13941421
};
13951422
if (this.data.appIdentifier) {
1423+
// A `Delete` operation is applied only after the deduplication runs, and no
1424+
// installation matched here to take a scope from. Skip the cleanup rather than
1425+
// run it unscoped across every application, or query on the operation itself.
1426+
if (typeof this.data.appIdentifier !== 'string') {
1427+
return;
1428+
}
13961429
delQuery['appIdentifier'] = this.data.appIdentifier;
13971430
}
13981431
this.config.database.destroy('_Installation', delQuery).catch(err => {
@@ -1452,7 +1485,19 @@ RestWrite.prototype.handleInstallation = function () {
14521485
return idMatch.objectId;
14531486
}
14541487
if (this.data.appIdentifier) {
1455-
delQuery['appIdentifier'] = this.data.appIdentifier;
1488+
// A `Delete` operation is applied only after the deduplication runs, so scope
1489+
// the cleanup to the value the matched installation still holds. Dropping the
1490+
// constraint would let the cleanup reach installations of other applications,
1491+
// and the operation itself cannot match a String, so skip the cleanup when no
1492+
// scope is available.
1493+
const appIdentifier =
1494+
typeof this.data.appIdentifier === 'string'
1495+
? this.data.appIdentifier
1496+
: idMatch.appIdentifier;
1497+
if (typeof appIdentifier !== 'string') {
1498+
return idMatch.objectId;
1499+
}
1500+
delQuery['appIdentifier'] = appIdentifier;
14561501
}
14571502
this.config.database.destroy('_Installation', delQuery).catch(err => {
14581503
if (err.code == Parse.Error.OBJECT_NOT_FOUND) {

0 commit comments

Comments
 (0)