Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 128 additions & 0 deletions spec/RequestComplexity.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,30 @@ describe('request complexity', () => {
return where;
}

function buildNestedOrQuery(depth) {
let where = { username: 'test' };
for (let i = 0; i < depth; i++) {
where = { $or: [where, { username: 'test' }] };
}
return where;
}

function buildNestedAndQuery(depth) {
let where = { username: 'test' };
for (let i = 0; i < depth; i++) {
where = { $and: [where, { username: 'test' }] };
}
return where;
}

function buildNestedNorQuery(depth) {
let where = { username: 'test' };
for (let i = 0; i < depth; i++) {
where = { $nor: [where, { username: 'test' }] };
}
return where;
}

describe('config validation', () => {
it('should accept valid requestComplexity config', async () => {
await expectAsync(
Expand All @@ -45,6 +69,7 @@ describe('request complexity', () => {
includeDepth: 10,
includeCount: 100,
subqueryDepth: 5,
queryDepth: 10,
graphQLDepth: 15,
graphQLFields: 300,
},
Expand All @@ -59,6 +84,7 @@ describe('request complexity', () => {
includeDepth: -1,
includeCount: -1,
subqueryDepth: -1,
queryDepth: -1,
graphQLDepth: -1,
graphQLFields: -1,
},
Expand Down Expand Up @@ -112,6 +138,7 @@ describe('request complexity', () => {
expect(config.requestComplexity.includeDepth).toBe(3);
expect(config.requestComplexity.includeCount).toBe(50);
expect(config.requestComplexity.subqueryDepth).toBe(5);
expect(config.requestComplexity.queryDepth).toBe(-1);
expect(config.requestComplexity.graphQLDepth).toBe(50);
expect(config.requestComplexity.graphQLFields).toBe(200);
});
Expand All @@ -123,6 +150,7 @@ describe('request complexity', () => {
includeDepth: 5,
includeCount: 50,
subqueryDepth: 5,
queryDepth: -1,
graphQLDepth: 50,
graphQLFields: 200,
});
Expand Down Expand Up @@ -216,6 +244,106 @@ describe('request complexity', () => {
});
});

describe('query depth', () => {
let config;

beforeEach(async () => {
await reconfigureServer({
requestComplexity: { queryDepth: 3 },
});
config = Config.get('test');
});

it('should allow $or within depth limit', async () => {
const where = buildNestedOrQuery(3);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeResolved();
});

it('should reject $or exceeding depth limit', async () => {
const where = buildNestedOrQuery(4);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringMatching(/Query condition nesting depth exceeds maximum allowed depth of 3/),
})
);
});

it('should reject $and exceeding depth limit', async () => {
const where = buildNestedAndQuery(4);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringMatching(/Query condition nesting depth exceeds maximum allowed depth of 3/),
})
);
});

it('should reject $nor exceeding depth limit', async () => {
const where = buildNestedNorQuery(4);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringMatching(/Query condition nesting depth exceeds maximum allowed depth of 3/),
})
);
});

it('should reject mixed nested operators exceeding depth limit', async () => {
// $or > $and > $nor > $or = depth 4
const where = {
$or: [
{
$and: [
{
$nor: [
{ $or: [{ username: 'a' }, { username: 'b' }] },
],
},
],
},
],
};
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringMatching(/Query condition nesting depth exceeds maximum allowed depth of 3/),
})
);
});

it('should allow with master key even when exceeding limit', async () => {
const where = buildNestedOrQuery(4);
await expectAsync(
rest.find(config, auth.master(config), '_User', where)
).toBeResolved();
});

it('should allow with maintenance key even when exceeding limit', async () => {
const where = buildNestedOrQuery(4);
await expectAsync(
rest.find(config, auth.maintenance(config), '_User', where)
).toBeResolved();
});

it('should allow unlimited when queryDepth is -1', async () => {
await reconfigureServer({
requestComplexity: { queryDepth: -1 },
});
config = Config.get('test');
const where = buildNestedOrQuery(15);
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeResolved();
});
});

describe('include limits', () => {
let config;

Expand Down
2 changes: 2 additions & 0 deletions spec/SecurityCheckGroups.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ describe('Security Check Groups', () => {
config.mountPlayground = false;
config.readOnlyMasterKey = 'someReadOnlyMasterKey';
config.readOnlyMasterKeyIps = ['127.0.0.1', '::1'];
config.requestComplexity = { queryDepth: 10 };
await reconfigureServer(config);

const group = new CheckGroupServerConfig();
Expand Down Expand Up @@ -66,6 +67,7 @@ describe('Security Check Groups', () => {
includeDepth: -1,
includeCount: -1,
subqueryDepth: -1,
queryDepth: -1,
graphQLDepth: -1,
graphQLFields: -1,
};
Expand Down
22 changes: 22 additions & 0 deletions spec/vulnerabilities.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2747,3 +2747,25 @@ describe('(GHSA-c442-97qw-j6c6) SQL Injection via $regex query operator field na
});
});
});

describe('(GHSA-9xp9-j92r-p88v) Stack overflow process crash via deeply nested query operators', () => {
it('rejects deeply nested $or query when queryDepth is set', async () => {
await reconfigureServer({
requestComplexity: { queryDepth: 10 },
});
const auth = require('../lib/Auth');
const rest = require('../lib/rest');
const config = Config.get('test');
let where = { username: 'test' };
for (let i = 0; i < 15; i++) {
where = { $or: [where, { username: 'test' }] };
}
await expectAsync(
rest.find(config, auth.nobody(config), '_User', where)
).toBeRejectedWith(
jasmine.objectContaining({
message: jasmine.stringMatching(/Query condition nesting depth exceeds maximum allowed depth/),
})
);
});
});
23 changes: 16 additions & 7 deletions src/Controllers/DatabaseController.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,34 +113,43 @@ const validateQuery = (
query: any,
isMaster: boolean,
isMaintenance: boolean,
update: boolean
update: boolean,
options: ?ParseServerOptions,
_depth: number = 0
): void => {
if (isMaintenance) {
isMaster = true;
}
const rc = options?.requestComplexity;
if (!isMaster && rc && rc.queryDepth !== -1 && _depth > rc.queryDepth) {
throw new Parse.Error(
Parse.Error.INVALID_QUERY,
`Query condition nesting depth exceeds maximum allowed depth of ${rc.queryDepth}`
);
}
if (query.ACL) {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Cannot query on ACL.');
}

if (query.$or) {
if (query.$or instanceof Array) {
query.$or.forEach(value => validateQuery(value, isMaster, isMaintenance, update));
query.$or.forEach(value => validateQuery(value, isMaster, isMaintenance, update, options, _depth + 1));
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $or format - use an array value.');
}
}

if (query.$and) {
if (query.$and instanceof Array) {
query.$and.forEach(value => validateQuery(value, isMaster, isMaintenance, update));
query.$and.forEach(value => validateQuery(value, isMaster, isMaintenance, update, options, _depth + 1));
} else {
throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Bad $and format - use an array value.');
}
}

if (query.$nor) {
if (query.$nor instanceof Array && query.$nor.length > 0) {
query.$nor.forEach(value => validateQuery(value, isMaster, isMaintenance, update));
query.$nor.forEach(value => validateQuery(value, isMaster, isMaintenance, update, options, _depth + 1));
} else {
throw new Parse.Error(
Parse.Error.INVALID_QUERY,
Expand Down Expand Up @@ -581,7 +590,7 @@ class DatabaseController {
if (acl) {
query = addWriteACL(query, acl);
}
validateQuery(query, isMaster, false, true);
validateQuery(query, isMaster, false, true, this.options);
return schemaController
.getOneSchema(className, true)
.catch(error => {
Expand Down Expand Up @@ -829,7 +838,7 @@ class DatabaseController {
if (acl) {
query = addWriteACL(query, acl);
}
validateQuery(query, isMaster, false, false);
validateQuery(query, isMaster, false, false, this.options);
return schemaController
.getOneSchema(className)
.catch(error => {
Expand Down Expand Up @@ -1340,7 +1349,7 @@ class DatabaseController {
query = addReadACL(query, aclGroup);
}
}
validateQuery(query, isMaster, isMaintenance, false);
validateQuery(query, isMaster, isMaintenance, false, this.options);
if (count) {
if (!classExists) {
return 0;
Expand Down
6 changes: 6 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,12 @@ module.exports.RequestComplexityOptions = {
action: parsers.numberParser('includeDepth'),
default: 5,
},
queryDepth: {
env: 'PARSE_SERVER_REQUEST_COMPLEXITY_QUERY_DEPTH',
help: 'Maximum nesting depth of `$or`, `$and`, `$nor` query operators. Set to `-1` to disable. Default is `-1`.',
action: parsers.numberParser('queryDepth'),
default: -1,
},
subqueryDepth: {
env: 'PARSE_SERVER_REQUEST_COMPLEXITY_SUBQUERY_DEPTH',
help: 'Maximum nesting depth of `$inQuery`, `$notInQuery`, `$select`, `$dontSelect` subqueries. Set to `-1` to disable. Default is `5`.',
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,9 @@ export interface RequestComplexityOptions {
/* Maximum nesting depth of `$inQuery`, `$notInQuery`, `$select`, `$dontSelect` subqueries. Set to `-1` to disable. Default is `5`.
:DEFAULT: 5 */
subqueryDepth: ?number;
/* Maximum nesting depth of `$or`, `$and`, `$nor` query operators. Set to `-1` to disable. Default is `-1`.
:DEFAULT: -1 */
queryDepth: ?number;
/* Maximum depth of GraphQL field selections. Set to `-1` to disable. Default is `50`.
:ENV: PARSE_SERVER_REQUEST_COMPLEXITY_GRAPHQL_DEPTH
:DEFAULT: 50 */
Expand Down
2 changes: 1 addition & 1 deletion src/Security/CheckGroups/CheckGroupServerConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class CheckGroupServerConfig extends CheckGroup {
if (!rc) {
throw 1;
}
const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.graphQLDepth, rc.graphQLFields];
const values = [rc.includeDepth, rc.includeCount, rc.subqueryDepth, rc.queryDepth, rc.graphQLDepth, rc.graphQLFields];
if (values.some(v => v === -1)) {
throw 1;
}
Expand Down
Loading