Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: sensitive keyword detection may produce false positives #7881

Merged
merged 2 commits into from
Mar 24, 2022
Merged

fix: sensitive keyword detection may produce false positives #7881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b279d73
fix string comparison
mtrezza Mar 23, 2022
640f926
fix and optimize keyword detection
mtrezza Mar 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions spec/vulnerabilities.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,4 +280,18 @@ describe('Vulnerabilities', () => {
expect(text.error).toBe('Prohibited keyword in request data: {"value":"aValue[123]*"}.');
});
});

describe('Ignore non-matches', () => {
it('ignores write request that contains only fraction of denied keyword', async () => {
await reconfigureServer({
requestKeywordDenylist: [{ key: 'abc' }],
});
// Initially saving an object executes the keyword detection in RestWrite.js
const obj = new TestObject({ a: { b: { c: 0 } } });
await expectAsync(obj.save()).toBeResolved();
// Modifying a nested key executes the keyword detection in DatabaseController.js
obj.increment('a.b.c');
await expectAsync(obj.save()).toBeResolved();
});
});
});
5 changes: 3 additions & 2 deletions src/Controllers/DatabaseController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import intersect from 'intersect';
// @flow-disable-next
import deepcopy from 'deepcopy';
import logger from '../logger';
import Utils from '../Utils';
import * as SchemaController from './SchemaController';
import { StorageAdapter } from '../Adapters/Storage/StorageAdapter';
import MongoStorageAdapter from '../Adapters/Storage/Mongo/MongoStorageAdapter';
Expand Down Expand Up @@ -1763,8 +1764,8 @@ class DatabaseController {
if (this.options && this.options.requestKeywordDenylist) {
// Scan request data for denied keywords
for (const keyword of this.options.requestKeywordDenylist) {
const isMatch = (a, b) => (typeof a === 'string' && new RegExp(a).test(b)) || a === b;
if (isMatch(firstKey, keyword.key)) {
const match = Utils.objectContainsKeyValue({ firstKey: undefined }, keyword.key, undefined);
if (match) {
throw new Parse.Error(
Parse.Error.INVALID_KEY_NAME,
`Prohibited keyword in request data: ${JSON.stringify(keyword)}.`
Expand Down
6 changes: 3 additions & 3 deletions src/Utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,9 @@ class Utils {
* @returns {Boolean} True if a match was found, false otherwise.
*/
static objectContainsKeyValue(obj, key, value) {
const isMatch = (a, b) => (typeof a === 'string' && new RegExp(a).test(b)) || a === b;
const isKeyMatch = k => isMatch(key, k);
const isValueMatch = v => isMatch(value, v);
const isMatch = (a, b) => (typeof a === 'string' && new RegExp(b).test(a)) || a === b;
const isKeyMatch = k => isMatch(k, key);
const isValueMatch = v => isMatch(v, value);
for (const [k, v] of Object.entries(obj)) {
if (key !== undefined && value === undefined && isKeyMatch(k)) {
return true;
Expand Down