Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-xqp8-w826-hh6x
* 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 <kartalkaanbozdogan@gmail.com>
Co-authored-by: Manuel <5673677+mtrezza@users.noreply.github.com>
  • Loading branch information
3 people committed Sep 2, 2021
1 parent 1e0d408 commit 308668c
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 4 deletions.
13 changes: 11 additions & 2 deletions CHANGELOG.md
Expand Up @@ -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) |
Expand Down Expand Up @@ -93,14 +94,16 @@ 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)
- Removed [parse-server-simple-mailgun-adapter](https://github.com/parse-community/parse-server-simple-mailgun-adapter) dependency; to continue using the adapter it has to be explicitly installed (Manuel Trezza) [#7321](https://github.com/parse-community/parse-server/pull/7321)
- 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)
Expand Down Expand Up @@ -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)

Expand Down
48 changes: 48 additions & 0 deletions spec/ParseQuery.spec.js
Expand Up @@ -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' });
Expand Down
19 changes: 19 additions & 0 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Expand Up @@ -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;
Expand Down Expand Up @@ -578,6 +595,7 @@ export class MongoStorageAdapter implements StorageAdapter {
query: QueryType,
{ skip, limit, sort, keys, readPreference, hint, caseInsensitive, explain }: QueryOptions
): Promise<any> {
validateExplainValue(explain);
schema = convertParseSchemaToMongoSchema(schema);
const mongoWhere = transformWhere(className, query, schema);
const mongoSort = _.mapKeys(sort, (value, fieldName) =>
Expand Down Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/RestQuery.js
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 308668c

Please sign in to comment.