From 6ae5835b1915c302ebfd56ca200f05c83241c162 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kartal=20Kaan=20Bozdo=C4=9Fan?= Date: Thu, 2 Sep 2021 12:46:48 +0200 Subject: [PATCH] Merge pull request from GHSA-xqp8-w826-hh6x * Backport the advisory fix * Added a 4.10.3 section to CHANGELOG --- CHANGELOG.md | 5 ++ spec/ParseQuery.spec.js | 48 +++++++++++++++++++ .../Storage/Mongo/MongoStorageAdapter.js | 19 ++++++++ src/RestQuery.js | 2 +- 4 files changed, 73 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b4a49e89..f8a2a543 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Parse Server Changelog +# 4.10.3 + +## Security Fixes +- Validate `explain` query parameter to avoid a server crash due to MongoDB bug [NODE-3463](https://jira.mongodb.org/browse/NODE-3463) (Kartal Kaan Bozdogan) [GHSA-xqp8-w826-hh6x](https://github.com/parse-community/parse-server/security/advisories/GHSA-xqp8-w826-hh6x) + # 4.10.2 [Full Changelog](https://github.com/parse-community/parse-server/compare/4.10.1...4.10.2) diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index 92daad50..91e2a2ac 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -33,6 +33,54 @@ describe('Parse.Query testing', () => { }); }); + it_only_db('mongo')('gracefully handles invalid explain values', async () => { + // Note that anything that is not truthy (like 0) does not cause an exception, as they get swallowed up by ClassesRouter::optionsFromBody + const values = [1, 'yolo', { a: 1 }, [1, 2, 3]]; + for (const value of values) { + try { + await request({ + method: 'GET', + url: `http://localhost:8378/1/classes/_User?explain=${value}`, + json: true, + headers: masterKeyHeaders, + }); + fail('request did not throw'); + } catch (e) { + // Expect that Parse Server did not crash + expect(e.code).not.toEqual('ECONNRESET'); + // Expect that Parse Server validates the explain value and does not crash; + // see https://jira.mongodb.org/browse/NODE-3463 + equal(e.data.code, Parse.Error.INVALID_QUERY); + equal(e.data.error, 'Invalid value for explain'); + } + // get queries (of the form '/classes/:className/:objectId' cannot have the explain key, see ClassesRouter.js) + // so it is enough that we test find queries + } + }); + + it_only_db('mongo')('supports valid explain values', async () => { + const values = [ + false, + true, + 'queryPlanner', + 'executionStats', + 'allPlansExecution', + // 'queryPlannerExtended' is excluded as it only applies to MongoDB Data Lake which is currently not available in our CI environment + ]; + for (const value of values) { + const response = await request({ + method: 'GET', + url: `http://localhost:8378/1/classes/_User?explain=${value}`, + json: true, + headers: masterKeyHeaders, + }); + expect(response.status).toBe(200); + if (value) { + expect(response.data.results.ok).toBe(1); + } + } + }); + it('searching for null', function (done) { const baz = new TestObject({ foo: null }); const qux = new TestObject({ foo: 'qux' }); diff --git a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js index cac2f661..59f00690 100644 --- a/src/Adapters/Storage/Mongo/MongoStorageAdapter.js +++ b/src/Adapters/Storage/Mongo/MongoStorageAdapter.js @@ -108,6 +108,23 @@ const mongoSchemaFromFieldsAndClassNameAndCLP = ( return mongoObject; }; +function validateExplainValue(explain) { + if (explain) { + // The list of allowed explain values is from node-mongodb-native/lib/explain.js + const explainAllowedValues = [ + 'queryPlanner', + 'queryPlannerExtended', + 'executionStats', + 'allPlansExecution', + false, + true, + ]; + if (!explainAllowedValues.includes(explain)) { + throw new Parse.Error(Parse.Error.INVALID_QUERY, 'Invalid value for explain'); + } + } +} + export class MongoStorageAdapter implements StorageAdapter { // Private _uri: string; @@ -562,6 +579,7 @@ export class MongoStorageAdapter implements StorageAdapter { query: QueryType, { skip, limit, sort, keys, readPreference, hint, caseInsensitive, explain }: QueryOptions ): Promise { + validateExplainValue(explain); schema = convertParseSchemaToMongoSchema(schema); const mongoWhere = transformWhere(className, query, schema); const mongoSort = _.mapKeys(sort, (value, fieldName) => @@ -740,6 +758,7 @@ export class MongoStorageAdapter implements StorageAdapter { hint: ?mixed, explain?: boolean ) { + validateExplainValue(explain); let isPointerField = false; pipeline = pipeline.map(stage => { if (stage.$group) { diff --git a/src/RestQuery.js b/src/RestQuery.js index 78fd022b..ab42ac95 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -633,7 +633,7 @@ RestQuery.prototype.runFind = function (options = {}) { return this.config.database .find(this.className, this.restWhere, findOptions, this.auth) .then(results => { - if (this.className === '_User' && findOptions.explain !== true) { + if (this.className === '_User' && !findOptions.explain) { for (var result of results) { cleanResultAuthData(result); }