diff --git a/packages/convict/src/main.js b/packages/convict/src/main.js index c0f9127..4e532af 100644 --- a/packages/convict/src/main.js +++ b/packages/convict/src/main.js @@ -9,6 +9,15 @@ const fs = require('fs') const parseArgs = require('yargs-parser') const cloneDeep = require('lodash.clonedeep') +// Forbidden key paths, for protection against prototype pollution +const FORBIDDEN_KEY_PATHS = [ + '__proto__', + 'this.constructor.prototype', +] + +const ALLOWED_OPTION_STRICT = 'strict' +const ALLOWED_OPTION_WARN = 'warn' + function assert(assertion, err_msg) { if (!assertion) { throw new Error(err_msg) @@ -69,9 +78,6 @@ const custom_converters = new Map() const parsers_registry = {'*': JSON.parse} -const ALLOWED_OPTION_STRICT = 'strict' -const ALLOWED_OPTION_WARN = 'warn' - function flatten(obj, useProperties) { const stack = Object.keys(obj) let key @@ -561,14 +567,18 @@ const convict = function convict(def, opts) { * exist, they will be initialized to empty objects */ set: function(k, v) { + for (const path of FORBIDDEN_KEY_PATHS) { + if (k.startsWith(`${path}.`)) { + return this + } + } + v = coerce(k, v, this._schema, this) const path = k.split('.') const childKey = path.pop() const parentKey = path.join('.') - if (!(parentKey == '__proto__' || parentKey == 'constructor' || parentKey == 'prototype')) { - const parent = walk(this._instance, parentKey, true) - parent[childKey] = v - } + const parent = walk(this._instance, parentKey, true) + parent[childKey] = v return this }, diff --git a/packages/convict/test/prototype_pollution.test.js b/packages/convict/test/prototype_pollution.test.js new file mode 100644 index 0000000..eb9466c --- /dev/null +++ b/packages/convict/test/prototype_pollution.test.js @@ -0,0 +1,31 @@ +'use strict' + +const convict = require('../') + +describe('Convict prototype pollution resistance', function() { + + test('against __proto__', function() { + const obj = {} + const config = convict(obj) + + config.set('__proto__.polluted_proto_root', 'Polluted!') + expect({}).not.toHaveProperty('polluted_proto_root') + + config.set('__proto__.nested.polluted_proto_nested', 'Polluted!') + expect({}).not.toHaveProperty('nested') + expect({}).not.toHaveProperty('nested.polluted_proto_nested') + }) + + test('against this.constructor.prototype', function() { + const obj = {} + const config = convict(obj) + + config.set('this.constructor.prototype.polluted_constructor_prototype_root', 'Polluted!') + expect({}).not.toHaveProperty('polluted_constructor_prototype_root') + + config.set('this.constructor.prototype.nested.polluted_constructor_prototype_nested', 'Polluted!') + expect({}).not.toHaveProperty('nested') + expect({}).not.toHaveProperty('nested.polluted_constructor_prototype_nested') + }) + +})