More complete fix against prototype pollution

first addressed in #384

Author: madarche

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
     },
 

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')
+  })
+
+})