CVE-2022-22143: Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution') (CWE-1321)
value: The package convict before 6.2.2 are vulnerable to Prototype Pollution via the convict function due to missing validation of parentKey. **Note:** This vulnerability derives from an incomplete fix of another [vulnerability](https://security.snyk.io/vuln/SNYK-JS-CONVICT-1062508)
CWE description: The product receives input from an upstream component that specifies attributes that are to be initialized or updated in an object, but it does not properly control modifications of attributes of the object prototype.
CVSS v3 base score: 9.8
Published: 2022-05-01T16:15Z
Repository: https://github.com/mozilla/node-convict
Language: JavaScript
Files changed: packages/convict/src/main.js, packages/convict/test/prototype_pollution.test.js
Commit date: 2022-03-27 11:48:47 +0200
Commit message: More complete fix against prototype pollution

first addressed in https://github.com/mozilla/node-convict/pull/384

Diff: 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
@@ -7,10 +7,19 @@
 
 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)
   }
 }
@@ -67,13 +76,10 @@ types.integer = types.int
 
 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
 
   const entries = []
@@ -559,18 +565,22 @@ const convict = function convict(def, opts) {
      * Sets the value of name to value. name can use dot notation to reference
      * nested values, e.g. "database.port". If objects in the chain don't yet
      * 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
     },
 
     /**
      * Loads and merges a JavaScript object into config
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')
+  })
+
+})

Vulnerable code: const ALLOWED_OPTION_STRICT = 'strict'
const ALLOWED_OPTION_WARN = 'warn'

      if (!(parentKey == '__proto__' || parentKey == 'constructor' || parentKey == 'prototype')) {
        const parent = walk(this._instance, parentKey, true)
        parent[childKey] = v
      }
Fixed code: // 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'

      for (const path of FORBIDDEN_KEY_PATHS) {
        if (k.startsWith(`${path}.`)) {
          return this
        }
      }

      const parent = walk(this._instance, parentKey, true)
      parent[childKey] = v
'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')
  })

})