From 308668c89474223e2448be92d6823b52c1c313ec Mon Sep 17 00:00:00 2001 From: Antonio Davi Macedo Coelho de Castro Date: Thu, 2 Sep 2021 03:46:48 -0700 Subject: [PATCH] Merge pull request from GHSA-xqp8-w826-hh6x MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Added a test case that triggers the query parameter crash * rest.js: validate the explain parameter to keep the nodejs driver from throwing an uncatchable exception and crashing the server (see https://jira.mongodb.org/browse/NODE-3463) RestQuery.js: Check whether explain mode is enabled not by "!== true", but by the "!" operator. explain can have string values. Added tests that validate correct behaviour on different explain values * Refactor the new tests * Simplify the new tests Also do a sanity check on the explain results * Test refactor * Exclude queryPlannerExtended as it is not supported by the testing environment Simplifies the tests * Restrict the changes to mongodb Moved the verification of the explain value from rest.js to MongoStorageAdapter.js Also restricted the relevant unit tests to mongodb * Added changelog entry * reformat changelog entry * Update CHANGELOG.md Co-authored-by: Kartal Kaan Bozdoğan Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com> --- CHANGELOG.md | 13 ++++- spec/ParseQuery.spec.js | 48 +++++++++++++++++++ .../Storage/Mongo/MongoStorageAdapter.js | 19 ++++++++ src/RestQuery.js | 4 +- 4 files changed, 80 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b04fa45b..0a2760bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,8 @@ Jump directly to a version: | 4.x | |--------------------------------------| -| [**4.10.2 (latest release)**](#4102) | +| [**4.10.3 (latest release)**](#4103) | +| [4.10.2](#4102) | | [4.10.1](#4101) | | [4.10.0](#4100) | | [4.5.2](#452) | @@ -93,7 +94,8 @@ Jump directly to a version: ___ ## Unreleased (Master Branch) -[Full Changelog](https://github.com/parse-community/parse-server/compare/4.10.2...master) +[Full Changelog](https://github.com/parse-community/parse-server/compare/4.10.3...master) + ### Breaking Changes - Improved schema caching through database real-time hooks. Reduces DB queries, decreases Parse Query execution time and fixes a potential schema memory leak. If multiple Parse Server instances connect to the same DB (for example behind a load balancer), set the [Parse Server Option](https://parseplatform.org/parse-server/api/master/ParseServerOptions.html) `databaseOptions.enableSchemaHooks: true` to enable this feature and keep the schema in sync across all instances. Failing to do so will cause a schema change to not propagate to other instances and re-syncing will only happen when these instances restart. The options `enableSingleSchemaCache` and `schemaCacheTTL` have been removed. To use this feature with MongoDB, a replica set cluster with [change stream](https://docs.mongodb.com/manual/changeStreams/#availability) support is required. (Diamond Lewis, SebC) [#7214](https://github.com/parse-community/parse-server/issues/7214) - Added file upload restriction. File upload is now only allowed for authenticated users by default for improved security. To allow file upload also for Anonymous Users or Public, set the `fileUpload` parameter in the [Parse Server Options](https://parseplatform.org/parse-server/api/master/ParseServerOptions.html) (dblythy, Manuel Trezza) [#7071](https://github.com/parse-community/parse-server/pull/7071) @@ -101,6 +103,7 @@ ___ - Remove support for MongoDB 3.6 which has reached its End-of-Life date and PostgreSQL 10 (Manuel Trezza) [#7315](https://github.com/parse-community/parse-server/pull/7315) - Remove support for Node 10 which has reached its End-of-Life date (Manuel Trezza) [#7314](https://github.com/parse-community/parse-server/pull/7314) - Remove S3 Files Adapter from Parse Server, instead install separately as `@parse/s3-files-adapter` (Manuel Trezza) [#7324](https://github.com/parse-community/parse-server/pull/7324) + ### Notable Changes - Added Parse Server Security Check to report weak security settings (Manuel Trezza, dblythy) [#7247](https://github.com/parse-community/parse-server/issues/7247) - EXPERIMENTAL: Added new page router with placeholder rendering and localization of custom and feature pages such as password reset and email verification (Manuel Trezza) [#7128](https://github.com/parse-community/parse-server/pull/7128) @@ -147,6 +150,12 @@ ___ - Add CI check to add changelog entry (Manuel Trezza) [#7512](https://github.com/parse-community/parse-server/pull/7512) - Refactor: uniform issue templates across repos (Manuel Trezza) [#7528](https://github.com/parse-community/parse-server/pull/7528) +## 4.10.3 +[Full Changelog](https://github.com/parse-community/parse-server/compare/4.10.2...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 e196280a..bf870c92 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -37,6 +37,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 2b5eaa0f..39cbcb5d 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; @@ -578,6 +595,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) => @@ -756,6 +774,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 6039084e..a919d509 100644 --- a/src/RestQuery.js +++ b/src/RestQuery.js @@ -657,7 +657,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); } @@ -866,7 +866,7 @@ function includePath(config, auth, response, path, restOptions = {}) { return set; } } - if (i == (keyPath.length - 1)) { + if (i == keyPath.length - 1) { set.add(keyPath[i]); } return set;