Skip to content

Commit

Permalink
More complete fix against prototype pollution
Browse files Browse the repository at this point in the history
first addressed in #384
  • Loading branch information
madarche committed Mar 27, 2022
1 parent e3173b1 commit 3b86be0
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 7 deletions.
24 changes: 17 additions & 7 deletions packages/convict/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
},

Expand Down
31 changes: 31 additions & 0 deletions packages/convict/test/prototype_pollution.test.js
Original file line number Diff line number Diff line change
@@ -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')
})

})

0 comments on commit 3b86be0

Please sign in to comment.