Skip to content

Commit

Permalink
Merge pull request from GHSA-xqp8-w826-hh6x
Browse files Browse the repository at this point in the history
* Backport the advisory fix

* Added a 4.10.3 section to CHANGELOG
  • Loading branch information
mstniy authored Sep 2, 2021
1 parent 0bfa6b7 commit 6ae5835
Show file tree
Hide file tree
Showing 4 changed files with 73 additions and 1 deletion.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)

Expand Down
48 changes: 48 additions & 0 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' });
Expand Down
19 changes: 19 additions & 0 deletions src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Original file line number Diff line number Diff line change
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 @@ -562,6 +579,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 @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/RestQuery.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 6ae5835

Please sign in to comment.