Skip to content

Commit 5365428

Browse files
mtrezzamtrezza
committed
fix: Input type validation for query operators and batch path
Add type validation to prevent uncaught TypeErrors from non-string inputs in query operators and batch subrequests, returning proper Parse errors instead of internal server errors. - Validate `$regex` is a string in `DatabaseController.validateQuery` - Validate `$options` is a string in `DatabaseController.validateQuery` - Catch invalid JSON in `where` parameter in `ClassesRouter` and `AggregateRouter` - Validate batch subrequest `path` is a string in `batch.js` - Validate `$in` element types match field schema type in Postgres adapter
1 parent 206e8b6 commit 5365428

File tree

7 files changed

+305
-2
lines changed

7 files changed

+305
-2
lines changed

‎spec/ParseQuery.spec.js‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5565,4 +5565,188 @@ describe('Parse.Query testing', () => {
55655565
}
55665566
);
55675567
});
5568+
5569+
describe('query input type validation', () => {
5570+
const restHeaders = {
5571+
'Content-Type': 'application/json',
5572+
'X-Parse-Application-Id': 'test',
5573+
'X-Parse-REST-API-Key': 'rest',
5574+
};
5575+
5576+
describe('malformed where parameter', () => {
5577+
it('rejects invalid JSON in where parameter with proper error instead of 500', async () => {
5578+
await expectAsync(
5579+
request({
5580+
method: 'GET',
5581+
url: 'http://localhost:8378/1/classes/TestClass?where=%7Bbad-json',
5582+
headers: restHeaders,
5583+
})
5584+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5585+
});
5586+
5587+
it('rejects invalid JSON in where parameter for roles endpoint', async () => {
5588+
await expectAsync(
5589+
request({
5590+
method: 'GET',
5591+
url: 'http://localhost:8378/1/roles?where=%7Bbad-json',
5592+
headers: restHeaders,
5593+
})
5594+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5595+
});
5596+
5597+
it('rejects invalid JSON in where parameter for users endpoint', async () => {
5598+
await expectAsync(
5599+
request({
5600+
method: 'GET',
5601+
url: 'http://localhost:8378/1/users?where=%7Bbad-json',
5602+
headers: restHeaders,
5603+
})
5604+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5605+
});
5606+
5607+
it('rejects invalid JSON in where parameter for sessions endpoint', async () => {
5608+
await expectAsync(
5609+
request({
5610+
method: 'GET',
5611+
url: 'http://localhost:8378/1/sessions?where=%7Bbad-json',
5612+
headers: restHeaders,
5613+
})
5614+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5615+
});
5616+
5617+
it('still accepts valid JSON in where parameter', async () => {
5618+
const result = await request({
5619+
method: 'GET',
5620+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(JSON.stringify({}))}`,
5621+
headers: restHeaders,
5622+
});
5623+
expect(result.data.results).toEqual([]);
5624+
});
5625+
});
5626+
5627+
describe('$regex type validation', () => {
5628+
it('rejects object $regex in query', async () => {
5629+
const where = JSON.stringify({ field: { $regex: { a: 1 } } });
5630+
await expectAsync(
5631+
request({
5632+
method: 'GET',
5633+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5634+
headers: restHeaders,
5635+
})
5636+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5637+
});
5638+
5639+
it('rejects numeric $regex in query', async () => {
5640+
const where = JSON.stringify({ field: { $regex: 123 } });
5641+
await expectAsync(
5642+
request({
5643+
method: 'GET',
5644+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5645+
headers: restHeaders,
5646+
})
5647+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5648+
});
5649+
5650+
it('rejects array $regex in query', async () => {
5651+
const where = JSON.stringify({ field: { $regex: ['test'] } });
5652+
await expectAsync(
5653+
request({
5654+
method: 'GET',
5655+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5656+
headers: restHeaders,
5657+
})
5658+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5659+
});
5660+
5661+
it('still accepts valid string $regex in query', async () => {
5662+
const obj = new Parse.Object('TestClass');
5663+
obj.set('field', 'hello');
5664+
await obj.save();
5665+
const where = JSON.stringify({ field: { $regex: '^hello$' } });
5666+
const result = await request({
5667+
method: 'GET',
5668+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5669+
headers: restHeaders,
5670+
});
5671+
expect(result.data.results.length).toBe(1);
5672+
});
5673+
});
5674+
5675+
describe('$options type validation', () => {
5676+
it('rejects numeric $options in query', async () => {
5677+
const where = JSON.stringify({ field: { $regex: 'test', $options: 123 } });
5678+
await expectAsync(
5679+
request({
5680+
method: 'GET',
5681+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5682+
headers: restHeaders,
5683+
})
5684+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5685+
});
5686+
5687+
it('rejects object $options in query', async () => {
5688+
const where = JSON.stringify({ field: { $regex: 'test', $options: { a: 1 } } });
5689+
await expectAsync(
5690+
request({
5691+
method: 'GET',
5692+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5693+
headers: restHeaders,
5694+
})
5695+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5696+
});
5697+
5698+
it('rejects boolean $options in query', async () => {
5699+
const where = JSON.stringify({ field: { $regex: 'test', $options: true } });
5700+
await expectAsync(
5701+
request({
5702+
method: 'GET',
5703+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5704+
headers: restHeaders,
5705+
})
5706+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5707+
});
5708+
5709+
it('still accepts valid string $options in query', async () => {
5710+
const obj = new Parse.Object('TestClass');
5711+
obj.set('field', 'hello');
5712+
await obj.save();
5713+
const where = JSON.stringify({ field: { $regex: 'hello', $options: 'i' } });
5714+
const result = await request({
5715+
method: 'GET',
5716+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5717+
headers: restHeaders,
5718+
});
5719+
expect(result.data.results.length).toBe(1);
5720+
});
5721+
});
5722+
5723+
describe('$in type validation on text fields', () => {
5724+
it_only_db('postgres')('rejects non-matching type in $in for text field with proper error instead of 500', async () => {
5725+
const obj = new Parse.Object('TestClass');
5726+
obj.set('textField', 'hello');
5727+
await obj.save();
5728+
const where = JSON.stringify({ textField: { $in: [1, 2, 3] } });
5729+
await expectAsync(
5730+
request({
5731+
method: 'GET',
5732+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5733+
headers: restHeaders,
5734+
})
5735+
).toBeRejectedWith(jasmine.objectContaining({ status: 400 }));
5736+
});
5737+
5738+
it('still accepts matching type in $in for text field', async () => {
5739+
const obj = new Parse.Object('TestClass');
5740+
obj.set('textField', 'hello');
5741+
await obj.save();
5742+
const where = JSON.stringify({ textField: { $in: ['hello', 'world'] } });
5743+
const result = await request({
5744+
method: 'GET',
5745+
url: `http://localhost:8378/1/classes/TestClass?where=${encodeURIComponent(where)}`,
5746+
headers: restHeaders,
5747+
});
5748+
expect(result.data.results.length).toBe(1);
5749+
});
5750+
});
5751+
});
55685752
});

‎spec/batch.spec.js‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,4 +593,93 @@ describe('batch', () => {
593593
});
594594
});
595595
}
596+
597+
describe('subrequest path type validation', () => {
598+
it('rejects object path in batch subrequest with proper error instead of 500', async () => {
599+
await expectAsync(
600+
request({
601+
method: 'POST',
602+
url: 'http://localhost:8378/1/batch',
603+
headers,
604+
body: JSON.stringify({
605+
requests: [{ method: 'GET', path: { invalid: true } }],
606+
}),
607+
})
608+
).toBeRejectedWith(
609+
jasmine.objectContaining({ status: 400 })
610+
);
611+
});
612+
613+
it('rejects numeric path in batch subrequest', async () => {
614+
await expectAsync(
615+
request({
616+
method: 'POST',
617+
url: 'http://localhost:8378/1/batch',
618+
headers,
619+
body: JSON.stringify({
620+
requests: [{ method: 'GET', path: 123 }],
621+
}),
622+
})
623+
).toBeRejectedWith(
624+
jasmine.objectContaining({ status: 400 })
625+
);
626+
});
627+
628+
it('rejects array path in batch subrequest', async () => {
629+
await expectAsync(
630+
request({
631+
method: 'POST',
632+
url: 'http://localhost:8378/1/batch',
633+
headers,
634+
body: JSON.stringify({
635+
requests: [{ method: 'GET', path: ['/1/classes/Test'] }],
636+
}),
637+
})
638+
).toBeRejectedWith(
639+
jasmine.objectContaining({ status: 400 })
640+
);
641+
});
642+
643+
it('rejects null path in batch subrequest', async () => {
644+
await expectAsync(
645+
request({
646+
method: 'POST',
647+
url: 'http://localhost:8378/1/batch',
648+
headers,
649+
body: JSON.stringify({
650+
requests: [{ method: 'GET', path: null }],
651+
}),
652+
})
653+
).toBeRejectedWith(
654+
jasmine.objectContaining({ status: 400 })
655+
);
656+
});
657+
658+
it('rejects boolean path in batch subrequest', async () => {
659+
await expectAsync(
660+
request({
661+
method: 'POST',
662+
url: 'http://localhost:8378/1/batch',
663+
headers,
664+
body: JSON.stringify({
665+
requests: [{ method: 'GET', path: true }],
666+
}),
667+
})
668+
).toBeRejectedWith(
669+
jasmine.objectContaining({ status: 400 })
670+
);
671+
});
672+
673+
it('still accepts valid string path in batch subrequest', async () => {
674+
const result = await request({
675+
method: 'POST',
676+
url: 'http://localhost:8378/1/batch',
677+
headers,
678+
body: JSON.stringify({
679+
requests: [{ method: 'GET', path: '/1/classes/TestClass' }],
680+
}),
681+
});
682+
expect(result.data).toEqual(jasmine.any(Array));
683+
});
684+
});
596685
});

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,17 @@ const buildWhereClause = ({ schema, query, index, caseInsensitive }): WhereClaus
486486
if (fieldName.indexOf('.') >= 0) {
487487
return;
488488
}
489+
const fieldType = schema.fields[fieldName]?.type;
490+
if (fieldType === 'String') {
491+
for (const elem of baseArray) {
492+
if (elem != null && typeof elem !== 'string') {
493+
throw new Parse.Error(
494+
Parse.Error.INVALID_QUERY,
495+
`$in element type mismatch: expected string for field "${fieldName}"`
496+
);
497+
}
498+
}
499+
}
489500
const inPatterns = [];
490501
values.push(fieldName);
491502
baseArray.forEach((listElem, listIndex) => {

‎src/Controllers/DatabaseController.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,12 @@ const validateQuery = (
160160

161161
Object.keys(query).forEach(key => {
162162
if (query && query[key] && query[key].$regex) {
163+
if (typeof query[key].$regex !== 'string') {
164+
throw new Parse.Error(Parse.Error.INVALID_QUERY, '$regex value must be a string');
165+
}
166+
if (query[key].$options !== undefined && typeof query[key].$options !== 'string') {
167+
throw new Parse.Error(Parse.Error.INVALID_QUERY, '$options value must be a string');
168+
}
163169
if (typeof query[key].$options === 'string') {
164170
if (!query[key].$options.match(/^[imxsu]+$/)) {
165171
throw new Parse.Error(

‎src/Routers/AggregateRouter.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ export class AggregateRouter extends ClassesRouter {
2929
}
3030
options.pipeline = AggregateRouter.getPipeline(body);
3131
if (typeof body.where === 'string') {
32-
body.where = JSON.parse(body.where);
32+
try {
33+
body.where = JSON.parse(body.where);
34+
} catch {
35+
throw new Parse.Error(Parse.Error.INVALID_JSON, 'where parameter is not valid JSON');
36+
}
3337
}
3438
try {
3539
const response = await rest.find(

‎src/Routers/ClassesRouter.js‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ export class ClassesRouter extends PromiseRouter {
3030
options.redirectClassNameForKey = String(body.redirectClassNameForKey);
3131
}
3232
if (typeof body.where === 'string') {
33-
body.where = JSON.parse(body.where);
33+
try {
34+
body.where = JSON.parse(body.where);
35+
} catch {
36+
throw new Parse.Error(Parse.Error.INVALID_JSON, 'where parameter is not valid JSON');
37+
}
3438
}
3539
return rest
3640
.find(

‎src/batch.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,11 @@ async function handleBatch(router, req) {
6767
if (!Array.isArray(req.body?.requests)) {
6868
throw new Parse.Error(Parse.Error.INVALID_JSON, 'requests must be an array');
6969
}
70+
for (const restRequest of req.body.requests) {
71+
if (typeof restRequest.path !== 'string') {
72+
throw new Parse.Error(Parse.Error.INVALID_JSON, 'batch request path must be a string');
73+
}
74+
}
7075

7176
// The batch paths are all from the root of our domain.
7277
// That means they include the API prefix, that the API is mounted

0 commit comments

Comments
 (0)
⚡