diff --git a/.madgerc b/.madgerc new file mode 100644 index 0000000000..4b9aa24bc2 --- /dev/null +++ b/.madgerc @@ -0,0 +1,10 @@ +{ + "detectiveOptions": { + "ts": { + "skipTypeImports": true + }, + "es6": { + "skipTypeImports": true + } + } +} diff --git a/resources/buildConfigDefinitions.js b/resources/buildConfigDefinitions.js index 52ff3ac7a8..670aafad38 100644 --- a/resources/buildConfigDefinitions.js +++ b/resources/buildConfigDefinitions.js @@ -172,6 +172,20 @@ function parseDefaultValue(elt, value, t) { literalValue = t.arrayExpression(array.map((value) => { if (typeof value == 'string') { return t.stringLiteral(value); + } else if (typeof value == 'number') { + return t.numericLiteral(value); + } else if (typeof value == 'object') { + const object = parsers.objectParser(value); + const props = Object.entries(object).map(([k, v]) => { + if (typeof v == 'string') { + return t.objectProperty(t.identifier(k), t.stringLiteral(v)); + } else if (typeof v == 'number') { + return t.objectProperty(t.identifier(k), t.numericLiteral(v)); + } else if (typeof v == 'boolean') { + return t.objectProperty(t.identifier(k), t.booleanLiteral(v)); + } + }); + return t.objectExpression(props); } else { throw new Error('Unable to parse array'); } diff --git a/spec/vulnerabilities.spec.js b/spec/vulnerabilities.spec.js new file mode 100644 index 0000000000..1255d64398 --- /dev/null +++ b/spec/vulnerabilities.spec.js @@ -0,0 +1,283 @@ +const request = require('../lib/request'); + +describe('Vulnerabilities', () => { + describe('Object prototype pollution', () => { + it('denies object prototype to be polluted with keyword "constructor"', async () => { + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const response = await request({ + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/PP', + body: JSON.stringify({ + obj: { + constructor: { + prototype: { + dummy: 0, + }, + }, + }, + }), + }).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe('Prohibited keyword in request data: {"key":"constructor"}.'); + expect(Object.prototype.dummy).toBeUndefined(); + }); + + it('denies object prototype to be polluted with keypath string "constructor"', async () => { + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const objResponse = await request({ + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/PP', + body: JSON.stringify({ + obj: {}, + }), + }).catch(e => e); + const pollResponse = await request({ + headers: headers, + method: 'PUT', + url: `http://localhost:8378/1/classes/PP/${objResponse.data.objectId}`, + body: JSON.stringify({ + 'obj.constructor.prototype.dummy': { + __op: 'Increment', + amount: 1, + }, + }), + }).catch(e => e); + expect(Object.prototype.dummy).toBeUndefined(); + expect(pollResponse.status).toBe(400); + const text = JSON.parse(pollResponse.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe('Prohibited keyword in request data: {"key":"constructor"}.'); + expect(Object.prototype.dummy).toBeUndefined(); + }); + + it('denies object prototype to be polluted with keyword "__proto__"', async () => { + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const response = await request({ + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/PP', + body: JSON.stringify({ 'obj.__proto__.dummy': 0 }), + }).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe('Prohibited keyword in request data: {"key":"__proto__"}.'); + expect(Object.prototype.dummy).toBeUndefined(); + }); + }); + + describe('Request denylist', () => { + it('denies BSON type code data in write request by default', async () => { + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + _bsontype: 'Code', + code: 'delete Object.prototype.evalFunctions', + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe( + 'Prohibited keyword in request data: {"key":"_bsontype","value":"Code"}.' + ); + }); + + it('allows BSON type code data in write request with custom denylist', async () => { + await reconfigureServer({ + requestKeywordDenylist: [], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + _bsontype: 'Code', + code: 'delete Object.prototype.evalFunctions', + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(201); + const text = JSON.parse(response.text); + expect(text.objectId).toBeDefined(); + }); + + it('denies write request with custom denylist of key/value', async () => { + await reconfigureServer({ + requestKeywordDenylist: [{ key: 'a[K]ey', value: 'aValue[123]*' }], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + aKey: 'aValue321', + code: 'delete Object.prototype.evalFunctions', + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe( + 'Prohibited keyword in request data: {"key":"a[K]ey","value":"aValue[123]*"}.' + ); + }); + + it('denies write request with custom denylist of nested key/value', async () => { + await reconfigureServer({ + requestKeywordDenylist: [{ key: 'a[K]ey', value: 'aValue[123]*' }], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + nested: { + aKey: 'aValue321', + code: 'delete Object.prototype.evalFunctions', + }, + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe( + 'Prohibited keyword in request data: {"key":"a[K]ey","value":"aValue[123]*"}.' + ); + }); + + it('denies write request with custom denylist of key/value in array', async () => { + await reconfigureServer({ + requestKeywordDenylist: [{ key: 'a[K]ey', value: 'aValue[123]*' }], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: [ + { + aKey: 'aValue321', + code: 'delete Object.prototype.evalFunctions', + }, + ], + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe( + 'Prohibited keyword in request data: {"key":"a[K]ey","value":"aValue[123]*"}.' + ); + }); + + it('denies write request with custom denylist of key', async () => { + await reconfigureServer({ + requestKeywordDenylist: [{ key: 'a[K]ey' }], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + aKey: 'aValue321', + code: 'delete Object.prototype.evalFunctions', + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe('Prohibited keyword in request data: {"key":"a[K]ey"}.'); + }); + + it('denies write request with custom denylist of value', async () => { + await reconfigureServer({ + requestKeywordDenylist: [{ value: 'aValue[123]*' }], + }); + const headers = { + 'Content-Type': 'application/json', + 'X-Parse-Application-Id': 'test', + 'X-Parse-REST-API-Key': 'rest', + }; + const params = { + headers: headers, + method: 'POST', + url: 'http://localhost:8378/1/classes/RCE', + body: JSON.stringify({ + obj: { + aKey: 'aValue321', + code: 'delete Object.prototype.evalFunctions', + }, + }), + }; + const response = await request(params).catch(e => e); + expect(response.status).toBe(400); + const text = JSON.parse(response.text); + expect(text.code).toBe(Parse.Error.INVALID_KEY_NAME); + expect(text.error).toBe('Prohibited keyword in request data: {"value":"aValue[123]*"}.'); + }); + }); +}); diff --git a/src/Config.js b/src/Config.js index 069b7e2a43..04834d3291 100644 --- a/src/Config.js +++ b/src/Config.js @@ -35,7 +35,7 @@ export class Config { config.applicationId = applicationId; Object.keys(cacheInfo).forEach(key => { if (key == 'databaseController') { - config.database = new DatabaseController(cacheInfo.databaseController.adapter); + config.database = new DatabaseController(cacheInfo.databaseController.adapter, config); } else { config[key] = cacheInfo[key]; } @@ -78,6 +78,7 @@ export class Config { security, enforcePrivateUsers, schema, + requestKeywordDenylist, }) { if (masterKey === readOnlyMasterKey) { throw new Error('masterKey and readOnlyMasterKey should be different'); @@ -116,6 +117,15 @@ export class Config { this.validateSecurityOptions(security); this.validateSchemaOptions(schema); this.validateEnforcePrivateUsers(enforcePrivateUsers); + this.validateRequestKeywordDenylist(requestKeywordDenylist); + } + + static validateRequestKeywordDenylist(requestKeywordDenylist) { + if (requestKeywordDenylist === undefined) { + requestKeywordDenylist = requestKeywordDenylist.default; + } else if (!Array.isArray(requestKeywordDenylist)) { + throw 'Parse Server option requestKeywordDenylist must be an array.'; + } } static validateEnforcePrivateUsers(enforcePrivateUsers) { diff --git a/src/Controllers/DatabaseController.js b/src/Controllers/DatabaseController.js index 2c313f83b4..f7322fc531 100644 --- a/src/Controllers/DatabaseController.js +++ b/src/Controllers/DatabaseController.js @@ -17,6 +17,7 @@ import MongoStorageAdapter from '../Adapters/Storage/Mongo/MongoStorageAdapter'; import PostgresStorageAdapter from '../Adapters/Storage/Postgres/PostgresStorageAdapter'; import SchemaCache from '../Adapters/Cache/SchemaCache'; import type { LoadSchemaOptions } from './types'; +import type { ParseServerOptions } from '../Options'; import type { QueryOptions, FullQueryOptions } from '../Adapters/Storage/StorageAdapter'; function addWriteACL(query, acl) { @@ -258,41 +259,6 @@ const isSpecialUpdateKey = key => { return specialKeysForUpdate.indexOf(key) >= 0; }; -function expandResultOnKeyPath(object, key, value) { - if (key.indexOf('.') < 0) { - object[key] = value[key]; - return object; - } - const path = key.split('.'); - const firstKey = path[0]; - const nextPath = path.slice(1).join('.'); - object[firstKey] = expandResultOnKeyPath(object[firstKey] || {}, nextPath, value[firstKey]); - delete object[key]; - return object; -} - -function sanitizeDatabaseResult(originalObject, result): Promise { - const response = {}; - if (!result) { - return Promise.resolve(response); - } - Object.keys(originalObject).forEach(key => { - const keyUpdate = originalObject[key]; - // determine if that was an op - if ( - keyUpdate && - typeof keyUpdate === 'object' && - keyUpdate.__op && - ['Add', 'AddUnique', 'Remove', 'Increment'].indexOf(keyUpdate.__op) > -1 - ) { - // only valid ops that produce an actionable result - // the op may have happend on a keypath - expandResultOnKeyPath(response, key, result); - } - }); - return Promise.resolve(response); -} - function joinTableName(className, key) { return `_Join:${key}:${className}`; } @@ -395,17 +361,18 @@ const relationSchema = { class DatabaseController { adapter: StorageAdapter; - idempotencyOptions: any; schemaCache: any; schemaPromise: ?Promise; _transactionalSession: ?any; + options: ParseServerOptions; + idempotencyOptions: any; - constructor(adapter: StorageAdapter, idempotencyOptions?: Object = {}) { + constructor(adapter: StorageAdapter, options: ParseServerOptions) { this.adapter = adapter; - this.idempotencyOptions = idempotencyOptions; - // We don't want a mutable this.schema, because then you could have - // one request that uses different schemas for different parts of - // it. Instead, use loadSchema to get a schema. + this.options = options || {}; + this.idempotencyOptions = this.options.idempotencyOptions || {}; + // Prevent mutable this.schema, otherwise one request could use + // multiple schemas, so instead use loadSchema to get a schema. this.schemaPromise = null; this._transactionalSession = null; } @@ -646,7 +613,7 @@ class DatabaseController { if (skipSanitization) { return Promise.resolve(result); } - return sanitizeDatabaseResult(originalUpdate, result); + return this._sanitizeDatabaseResult(originalUpdate, result); }); }); } @@ -873,7 +840,7 @@ class DatabaseController { object, relationUpdates ).then(() => { - return sanitizeDatabaseResult(originalObject, result.ops[0]); + return this._sanitizeDatabaseResult(originalObject, result.ops[0]); }); }); }); @@ -1782,6 +1749,60 @@ class DatabaseController { await this.adapter.updateSchemaWithIndexes(); } + _expandResultOnKeyPath(object: any, key: string, value: any): any { + if (key.indexOf('.') < 0) { + object[key] = value[key]; + return object; + } + const path = key.split('.'); + const firstKey = path[0]; + const nextPath = path.slice(1).join('.'); + + // Scan request data for denied keywords + 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)) { + throw new Parse.Error( + Parse.Error.INVALID_KEY_NAME, + `Prohibited keyword in request data: ${JSON.stringify(keyword)}.` + ); + } + } + } + + object[firstKey] = this._expandResultOnKeyPath( + object[firstKey] || {}, + nextPath, + value[firstKey] + ); + delete object[key]; + return object; + } + + _sanitizeDatabaseResult(originalObject: any, result: any): Promise { + const response = {}; + if (!result) { + return Promise.resolve(response); + } + Object.keys(originalObject).forEach(key => { + const keyUpdate = originalObject[key]; + // determine if that was an op + if ( + keyUpdate && + typeof keyUpdate === 'object' && + keyUpdate.__op && + ['Add', 'AddUnique', 'Remove', 'Increment'].indexOf(keyUpdate.__op) > -1 + ) { + // only valid ops that produce an actionable result + // the op may have happened on a keypath + this._expandResultOnKeyPath(response, key, result); + } + }); + return Promise.resolve(response); + } + static _validateQuery: any => void; } diff --git a/src/Controllers/index.js b/src/Controllers/index.js index 152de684fe..b2feff0fc2 100644 --- a/src/Controllers/index.js +++ b/src/Controllers/index.js @@ -142,7 +142,7 @@ export function getLiveQueryController(options: ParseServerOptions): LiveQueryCo } export function getDatabaseController(options: ParseServerOptions): DatabaseController { - const { databaseURI, collectionPrefix, databaseOptions, idempotencyOptions } = options; + const { databaseURI, collectionPrefix, databaseOptions } = options; let { databaseAdapter } = options; if ( (databaseOptions || @@ -156,7 +156,7 @@ export function getDatabaseController(options: ParseServerOptions): DatabaseCont } else { databaseAdapter = loadAdapter(databaseAdapter); } - return new DatabaseController(databaseAdapter, idempotencyOptions); + return new DatabaseController(databaseAdapter, options); } export function getHooksController( diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index e2cbd17920..1edd704bd6 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -350,6 +350,24 @@ module.exports.ParseServerOptions = { env: 'PARSE_SERVER_READ_ONLY_MASTER_KEY', help: 'Read-only key, which has the same capabilities as MasterKey without writes', }, + requestKeywordDenylist: { + env: 'PARSE_SERVER_REQUEST_KEYWORD_DENYLIST', + help: + 'An array of keys and values that are prohibited in database read and write requests to prevent potential security vulnerabilities. It is possible to specify only a key (`{"key":"..."}`), only a value (`{"value":"..."}`) or a key-value pair (`{"key":"...","value":"..."}`). The specification can use the following types: `boolean`, `numeric` or `string`, where `string` will be interpreted as a regex notation. Request data is deep-scanned for matching definitions to detect also any nested occurrences. Defaults are patterns that are likely to be used in malicious requests. Setting this option will override the default patterns.', + action: parsers.arrayParser, + default: [ + { + key: '_bsontype', + value: 'Code', + }, + { + key: 'constructor', + }, + { + key: '__proto__', + }, + ], + }, restAPIKey: { env: 'PARSE_SERVER_REST_API_KEY', help: 'Key for REST calls', diff --git a/src/Options/docs.js b/src/Options/docs.js index c6b2e1abf1..fc0ff3b799 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -64,6 +64,7 @@ * @property {String} publicServerURL Public URL to your parse server with http:// or https://. * @property {Any} push Configuration for push, as stringified JSON. See http://docs.parseplatform.org/parse-server/guide/#push-notifications * @property {String} readOnlyMasterKey Read-only key, which has the same capabilities as MasterKey without writes + * @property {RequestKeywordDenylist[]} requestKeywordDenylist An array of keys and values that are prohibited in database read and write requests to prevent potential security vulnerabilities. It is possible to specify only a key (`{"key":"..."}`), only a value (`{"value":"..."}`) or a key-value pair (`{"key":"...","value":"..."}`). The specification can use the following types: `boolean`, `numeric` or `string`, where `string` will be interpreted as a regex notation. Request data is deep-scanned for matching definitions to detect also any nested occurrences. Defaults are patterns that are likely to be used in malicious requests. Setting this option will override the default patterns. * @property {String} restAPIKey Key for REST calls * @property {Boolean} revokeSessionOnPasswordReset When a user changes their password, either through the reset password email or while logged in, all sessions are revoked if this is true. Set to false if you don't want to revoke sessions. * @property {Boolean} scheduledPush Configuration for push scheduling, defaults to false. diff --git a/src/Options/index.js b/src/Options/index.js index 31b9e10d41..3482d88c50 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -14,6 +14,10 @@ type Adapter = string | any | T; type NumberOrBoolean = number | boolean; type NumberOrString = number | string; type ProtectedFields = any; +type RequestKeywordDenylist = { + key: string | any, + value: any, +}; export interface ParseServerOptions { /* Your Parse Application ID @@ -252,6 +256,9 @@ export interface ParseServerOptions { /* Set to true if new users should be created without public read and write access. :DEFAULT: false */ enforcePrivateUsers: ?boolean; + /* An array of keys and values that are prohibited in database read and write requests to prevent potential security vulnerabilities. It is possible to specify only a key (`{"key":"..."}`), only a value (`{"value":"..."}`) or a key-value pair (`{"key":"...","value":"..."}`). The specification can use the following types: `boolean`, `numeric` or `string`, where `string` will be interpreted as a regex notation. Request data is deep-scanned for matching definitions to detect also any nested occurrences. Defaults are patterns that are likely to be used in malicious requests. Setting this option will override the default patterns. + :DEFAULT: [{"key":"_bsontype","value":"Code"},{"key":"constructor"},{"key":"__proto__"}] */ + requestKeywordDenylist: ?(RequestKeywordDenylist[]); } export interface SecurityOptions { diff --git a/src/RestWrite.js b/src/RestWrite.js index a651cf9c6c..8b728731da 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -6,6 +6,7 @@ var SchemaController = require('./Controllers/SchemaController'); var deepcopy = require('deepcopy'); const Auth = require('./Auth'); +const Utils = require('./Utils'); var cryptoUtils = require('./cryptoUtils'); var passwordCrypto = require('./password'); var Parse = require('parse/node'); @@ -61,6 +62,19 @@ function RestWrite(config, auth, className, query, data, originalData, clientSDK } } + if (this.config.requestKeywordDenylist) { + // Scan request data for denied keywords + for (const keyword of this.config.requestKeywordDenylist) { + const match = Utils.objectContainsKeyValue(data, keyword.key, keyword.value); + if (match) { + throw new Parse.Error( + Parse.Error.INVALID_KEY_NAME, + `Prohibited keyword in request data: ${JSON.stringify(keyword)}.` + ); + } + } + } + // When the operation is complete, this.response may have several // fields. // response: the actual data to be returned diff --git a/src/Utils.js b/src/Utils.js index c96be08495..399939a152 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -332,6 +332,32 @@ class Utils { }; } } + + /** + * Deep-scans an object for a matching key/value definition. + * @param {Object} obj The object to scan. + * @param {String | undefined} key The key to match, or undefined if only the value should be matched. + * @param {any | undefined} value The value to match, or undefined if only the key should be matched. + * @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); + for (const [k, v] of Object.entries(obj)) { + if (key !== undefined && value === undefined && isKeyMatch(k)) { + return true; + } else if (key === undefined && value !== undefined && isValueMatch(v)) { + return true; + } else if (key !== undefined && value !== undefined && isKeyMatch(k) && isValueMatch(v)) { + return true; + } + if (['[object Object]', '[object Array]'].includes(Object.prototype.toString.call(v))) { + return Utils.objectContainsKeyValue(v, key, value); + } + } + return false; + } } module.exports = Utils;