diff --git a/spec/RequestComplexity.spec.js b/spec/RequestComplexity.spec.js index 6ee159f548..2765ca02ec 100644 --- a/spec/RequestComplexity.spec.js +++ b/spec/RequestComplexity.spec.js @@ -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( @@ -45,6 +69,7 @@ describe('request complexity', () => { includeDepth: 10, includeCount: 100, subqueryDepth: 5, + queryDepth: 10, graphQLDepth: 15, graphQLFields: 300, }, @@ -59,6 +84,7 @@ describe('request complexity', () => { includeDepth: -1, includeCount: -1, subqueryDepth: -1, + queryDepth: -1, graphQLDepth: -1, graphQLFields: -1, }, @@ -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); }); @@ -123,6 +150,7 @@ describe('request complexity', () => { includeDepth: 5, includeCount: 50, subqueryDepth: 5, + queryDepth: -1, graphQLDepth: 50, graphQLFields: 200, }); @@ -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; diff --git a/spec/SecurityCheckGroups.spec.js b/spec/SecurityCheckGroups.spec.js index ba4ecd8be7..5bd137f032 100644 --- a/spec/SecurityCheckGroups.spec.js +++ b/spec/SecurityCheckGroups.spec.js @@ -34,6 +34,7 @@ describe('Security Check Groups', () => { config.allowClientClassCreation = false; config.enableInsecureAuthAdapters = false; config.graphQLPublicIntrospection = false; + config.requestComplexity = { queryDepth: 10 }; await reconfigureServer(config); const group = new CheckGroupServerConfig(); @@ -57,6 +58,7 @@ describe('Security Check Groups', () => { includeDepth: -1, includeCount: -1, subqueryDepth: -1, + queryDepth: -1, graphQLDepth: -1, graphQLFields: -1, }; diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js index fe22a6f03c..c0cae46131 100644 --- a/spec/vulnerabilities.spec.js +++ b/spec/vulnerabilities.spec.js @@ -2613,3 +2613,25 @@ describe('(GHSA-42ph-pf9q-cr72) Stored XSS filter bypass via parameterized Conte }); }); }); + +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/), + }) + ); + }); +}); diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 2e81eeccc8..2fb83b768c 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -73,18 +73,27 @@ 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.'); } @@ -92,7 +101,7 @@ const validateQuery = ( 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.'); } @@ -100,7 +109,7 @@ const validateQuery = ( 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, @@ -545,7 +554,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 => { @@ -793,7 +802,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 => { @@ -1298,7 +1307,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; diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index d0cf5f8ff1..17523183e2 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -745,6 +745,13 @@ 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: diff --git a/src/Options/docs.js b/src/Options/docs.js index 20a6f95abd..b72dce3698 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -134,6 +134,7 @@ * @property {Number} graphQLFields Maximum number of field selections in a GraphQL query. Set to `-1` to disable. Default is `200`. * @property {Number} includeCount Maximum number of include paths in a single query. Set to `-1` to disable. Default is `50`. * @property {Number} includeDepth Maximum depth of include pointer chains (e.g. `a.b.c` = depth 3). Set to `-1` to disable. Default is `5`. + * @property {Number} queryDepth Maximum nesting depth of `$or`, `$and`, `$nor` query operators. Set to `-1` to disable. Default is `-1`. * @property {Number} subqueryDepth Maximum nesting depth of `$inQuery`, `$notInQuery`, `$select`, `$dontSelect` subqueries. Set to `-1` to disable. Default is `5`. */ diff --git a/src/Options/index.js b/src/Options/index.js index fa8ba73b6c..70dcfcbc66 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -406,6 +406,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 */ diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js index 62e87738b2..e49f4edc8d 100644 --- a/src/Security/CheckGroups/CheckGroupServerConfig.js +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -116,7 +116,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; }