Skip to content

Commit 8af6e28

Browse files
mtrezzamtrezza
committed
fix: Prevent SQL injection via aggregate and distinct field names in PostgreSQL adapter
1 parent cdd3776 commit 8af6e28

File tree

3 files changed

+203
-2
lines changed

3 files changed

+203
-2
lines changed

‎spec/vulnerabilities.spec.js‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4206,3 +4206,187 @@ describe('(GHSA-g4cf-xj29-wqqr) DoS via unindexed database query for unconfigure
42064206
expect(authDataQueries.length).toBeGreaterThan(0);
42074207
});
42084208
});
4209+
4210+
describe('(GHSA-p2w6-rmh7-w8q3) SQL Injection via aggregate and distinct field names in PostgreSQL adapter', () => {
4211+
const headers = {
4212+
'Content-Type': 'application/json',
4213+
'X-Parse-Application-Id': 'test',
4214+
'X-Parse-REST-API-Key': 'rest',
4215+
'X-Parse-Master-Key': 'test',
4216+
};
4217+
const serverURL = 'http://localhost:8378/1';
4218+
4219+
beforeEach(async () => {
4220+
const obj = new Parse.Object('TestClass');
4221+
obj.set('playerName', 'Alice');
4222+
obj.set('score', 100);
4223+
obj.set('metadata', { tag: 'hello' });
4224+
await obj.save(null, { useMasterKey: true });
4225+
});
4226+
4227+
describe('aggregate $group._id SQL injection', () => {
4228+
it_only_db('postgres')('rejects $group._id field value containing double quotes', async () => {
4229+
const response = await request({
4230+
method: 'GET',
4231+
url: `${serverURL}/aggregate/TestClass`,
4232+
headers,
4233+
qs: {
4234+
pipeline: JSON.stringify([
4235+
{
4236+
$group: {
4237+
_id: {
4238+
alias: '$playerName" OR 1=1 --',
4239+
},
4240+
},
4241+
},
4242+
]),
4243+
},
4244+
}).catch(e => e);
4245+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4246+
});
4247+
4248+
it_only_db('postgres')('rejects $group._id field value containing semicolons', async () => {
4249+
const response = await request({
4250+
method: 'GET',
4251+
url: `${serverURL}/aggregate/TestClass`,
4252+
headers,
4253+
qs: {
4254+
pipeline: JSON.stringify([
4255+
{
4256+
$group: {
4257+
_id: {
4258+
alias: '$playerName"; DROP TABLE "TestClass" --',
4259+
},
4260+
},
4261+
},
4262+
]),
4263+
},
4264+
}).catch(e => e);
4265+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4266+
});
4267+
4268+
it_only_db('postgres')('rejects $group._id date operation field value containing double quotes', async () => {
4269+
const response = await request({
4270+
method: 'GET',
4271+
url: `${serverURL}/aggregate/TestClass`,
4272+
headers,
4273+
qs: {
4274+
pipeline: JSON.stringify([
4275+
{
4276+
$group: {
4277+
_id: {
4278+
day: { $dayOfMonth: '$createdAt" OR 1=1 --' },
4279+
},
4280+
},
4281+
},
4282+
]),
4283+
},
4284+
}).catch(e => e);
4285+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4286+
});
4287+
4288+
it_only_db('postgres')('allows legitimate $group._id with field reference', async () => {
4289+
const response = await request({
4290+
method: 'GET',
4291+
url: `${serverURL}/aggregate/TestClass`,
4292+
headers,
4293+
qs: {
4294+
pipeline: JSON.stringify([
4295+
{
4296+
$group: {
4297+
_id: {
4298+
name: '$playerName',
4299+
},
4300+
count: { $sum: 1 },
4301+
},
4302+
},
4303+
]),
4304+
},
4305+
});
4306+
expect(response.data?.results?.length).toBeGreaterThan(0);
4307+
});
4308+
4309+
it_only_db('postgres')('allows legitimate $group._id with date extraction', async () => {
4310+
const response = await request({
4311+
method: 'GET',
4312+
url: `${serverURL}/aggregate/TestClass`,
4313+
headers,
4314+
qs: {
4315+
pipeline: JSON.stringify([
4316+
{
4317+
$group: {
4318+
_id: {
4319+
day: { $dayOfMonth: '$_created_at' },
4320+
},
4321+
count: { $sum: 1 },
4322+
},
4323+
},
4324+
]),
4325+
},
4326+
});
4327+
expect(response.data?.results?.length).toBeGreaterThan(0);
4328+
});
4329+
});
4330+
4331+
describe('distinct dot-notation SQL injection', () => {
4332+
it_only_db('postgres')('rejects distinct field name containing double quotes in dot notation', async () => {
4333+
const response = await request({
4334+
method: 'GET',
4335+
url: `${serverURL}/aggregate/TestClass`,
4336+
headers,
4337+
qs: {
4338+
distinct: 'metadata" FROM pg_tables; --.tag',
4339+
},
4340+
}).catch(e => e);
4341+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4342+
});
4343+
4344+
it_only_db('postgres')('rejects distinct field name containing semicolons in dot notation', async () => {
4345+
const response = await request({
4346+
method: 'GET',
4347+
url: `${serverURL}/aggregate/TestClass`,
4348+
headers,
4349+
qs: {
4350+
distinct: 'metadata; DROP TABLE "TestClass" --.tag',
4351+
},
4352+
}).catch(e => e);
4353+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4354+
});
4355+
4356+
it_only_db('postgres')('rejects distinct field name containing single quotes in dot notation', async () => {
4357+
const response = await request({
4358+
method: 'GET',
4359+
url: `${serverURL}/aggregate/TestClass`,
4360+
headers,
4361+
qs: {
4362+
distinct: "metadata' OR '1'='1.tag",
4363+
},
4364+
}).catch(e => e);
4365+
expect(response.data?.code).toBe(Parse.Error.INVALID_KEY_NAME);
4366+
});
4367+
4368+
it_only_db('postgres')('allows legitimate distinct with dot notation', async () => {
4369+
const response = await request({
4370+
method: 'GET',
4371+
url: `${serverURL}/aggregate/TestClass`,
4372+
headers,
4373+
qs: {
4374+
distinct: 'metadata.tag',
4375+
},
4376+
});
4377+
expect(response.data?.results).toEqual(['hello']);
4378+
});
4379+
4380+
it_only_db('postgres')('allows legitimate distinct without dot notation', async () => {
4381+
const response = await request({
4382+
method: 'GET',
4383+
url: `${serverURL}/aggregate/TestClass`,
4384+
headers,
4385+
qs: {
4386+
distinct: 'playerName',
4387+
},
4388+
});
4389+
expect(response.data?.results).toEqual(['Alice']);
4390+
});
4391+
});
4392+
});

‎src/Adapters/Storage/Postgres/PostgresStorageAdapter.js‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,12 @@ const transformDotField = fieldName => {
234234
return name;
235235
};
236236

237+
const validateAggregateFieldName = name => {
238+
if (typeof name !== 'string' || !name.match(/^[a-zA-Z][a-zA-Z0-9_]*$/)) {
239+
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name: ${name}`);
240+
}
241+
};
242+
237243
const transformAggregateField = fieldName => {
238244
if (typeof fieldName !== 'string') {
239245
return fieldName;
@@ -244,7 +250,9 @@ const transformAggregateField = fieldName => {
244250
if (fieldName === '$_updated_at') {
245251
return 'updatedAt';
246252
}
247-
return fieldName.substring(1);
253+
const name = fieldName.substring(1);
254+
validateAggregateFieldName(name);
255+
return name;
248256
};
249257

250258
const validateKeys = object => {
@@ -2179,12 +2187,18 @@ export class PostgresStorageAdapter implements StorageAdapter {
21792187
21802188
async distinct(className: string, schema: SchemaType, query: QueryType, fieldName: string) {
21812189
debug('distinct');
2190+
const fieldSegments = fieldName.split('.');
2191+
for (const segment of fieldSegments) {
2192+
if (!segment.match(/^[a-zA-Z][a-zA-Z0-9_]*$/)) {
2193+
throw new Parse.Error(Parse.Error.INVALID_KEY_NAME, `Invalid field name: ${fieldName}`);
2194+
}
2195+
}
21822196
let field = fieldName;
21832197
let column = fieldName;
21842198
const isNested = fieldName.indexOf('.') >= 0;
21852199
if (isNested) {
21862200
field = transformDotFieldToComponents(fieldName).join('->');
2187-
column = fieldName.split('.')[0];
2201+
column = fieldSegments[0];
21882202
}
21892203
const isArrayField =
21902204
schema.fields && schema.fields[fieldName] && schema.fields[fieldName].type === 'Array';

‎src/Routers/AggregateRouter.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ export class AggregateRouter extends ClassesRouter {
5252
}
5353
return { response };
5454
} catch (e) {
55+
if (e instanceof Parse.Error) {
56+
throw e;
57+
}
5558
throw new Parse.Error(Parse.Error.INVALID_QUERY, e.message);
5659
}
5760
}

0 commit comments

Comments
 (0)
⚡