CVE-2022-21190: Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution') (CWE-1321)
value: This affects the package convict before 6.2.3. This is a bypass of [CVE-2022-22143](https://security.snyk.io/vuln/SNYK-JS-CONVICT-2340604). The [fix](https://github.com/mozilla/node-convict/commit/3b86be087d8f14681a9c889d45da7fe3ad9cd880) introduced, relies on the startsWith method and does not prevent the vulnerability: before splitting the path, it checks if it starts with __proto__ or this.constructor.prototype. To bypass this check it's possible to prepend the dangerous paths with any string value followed by a dot, like for example foo.__proto__ or foo.this.constructor.prototype.
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-13T20:15Z
Repository: https://github.com/mozilla/node-convict
Language: JavaScript
Files changed: CHANGELOG.md, packages/convict/src/main.js, packages/convict/test/prototype_pollution.test.js
Commit date: 2022-05-07 16:37:39 +0200
Commit message: More more complete fix for prototype pollution

vulnerability first addressed in #384

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2c20ff6..60fa7ae 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,10 +4,18 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
 
+## [6.2.3] - 2022-05-07
+
+### Fixed
+
+- More more complete fix for prototype pollution vulnerability first addressed
+  in #384 (Marc-Aurèle Darche @madarche, Snyk Security team)
+
+
 ## [6.2.2] - 2022-03-27
 
 ### Fixed
 
 - More complete fix for prototype pollution vulnerability first addressed in
diff --git a/packages/convict/src/main.js b/packages/convict/src/main.js
index 4e532af..136ce4c 100644
--- a/packages/convict/src/main.js
+++ b/packages/convict/src/main.js
@@ -9,12 +9,12 @@ 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',
+  '__proto__.',
+  'this.constructor.prototype.',
 ]
 
 const ALLOWED_OPTION_STRICT = 'strict'
 const ALLOWED_OPTION_WARN = 'warn'
 
@@ -565,12 +565,13 @@ 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}.`)) {
+      for (const forbidden_key_path of FORBIDDEN_KEY_PATHS) {
+        if (k.startsWith(forbidden_key_path) ||
+            k.includes(`.${forbidden_key_path}`)) {
           return this
         }
       }
 
       v = coerce(k, v, this._schema, this)
diff --git a/packages/convict/test/prototype_pollution.test.js b/packages/convict/test/prototype_pollution.test.js
index eb9466c..4578594 100644
--- a/packages/convict/test/prototype_pollution.test.js
+++ b/packages/convict/test/prototype_pollution.test.js
@@ -12,10 +12,24 @@ describe('Convict prototype pollution resistance', function() {
     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')
+
+    config.set('this.__proto__.polluted_proto_root', 'Polluted!')
+    expect({}).not.toHaveProperty('polluted_proto_root')
+
+    config.set('this.__proto__.nested.polluted_proto_nested', 'Polluted!')
+    expect({}).not.toHaveProperty('nested')
+    expect({}).not.toHaveProperty('nested.polluted_proto_nested')
+
+    config.set('foo.__proto__.polluted_proto_root', 'Polluted!')
+    expect({}).not.toHaveProperty('polluted_proto_root')
+
+    config.set('foo.__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)
@@ -24,8 +38,22 @@ describe('Convict prototype pollution resistance', function() {
     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')
+
+    config.set('this.this.constructor.prototype.polluted_constructor_prototype_root', 'Polluted!')
+    expect({}).not.toHaveProperty('polluted_constructor_prototype_root')
+
+    config.set('this.this.constructor.prototype.nested.polluted_constructor_prototype_nested', 'Polluted!')
+    expect({}).not.toHaveProperty('nested')
+    expect({}).not.toHaveProperty('nested.polluted_constructor_prototype_nested')
+
+    config.set('foo.this.constructor.prototype.polluted_constructor_prototype_root', 'Polluted!')
+    expect({}).not.toHaveProperty('polluted_constructor_prototype_root')
+
+    config.set('foo.this.constructor.prototype.nested.polluted_constructor_prototype_nested', 'Polluted!')
+    expect({}).not.toHaveProperty('nested')
+    expect({}).not.toHaveProperty('nested.polluted_constructor_prototype_nested')
   })
 
 })

Vulnerable code:   '__proto__',
  'this.constructor.prototype',
      for (const path of FORBIDDEN_KEY_PATHS) {
        if (k.startsWith(`${path}.`)) {
Fixed code: ## [6.2.3] - 2022-05-07

### Fixed

- More more complete fix for prototype pollution vulnerability first addressed
  in #384 (Marc-Aurèle Darche @madarche, Snyk Security team)


  '__proto__.',
  'this.constructor.prototype.',
      for (const forbidden_key_path of FORBIDDEN_KEY_PATHS) {
        if (k.startsWith(forbidden_key_path) ||
            k.includes(`.${forbidden_key_path}`)) {

    config.set('this.__proto__.polluted_proto_root', 'Polluted!')
    expect({}).not.toHaveProperty('polluted_proto_root')

    config.set('this.__proto__.nested.polluted_proto_nested', 'Polluted!')
    expect({}).not.toHaveProperty('nested')
    expect({}).not.toHaveProperty('nested.polluted_proto_nested')

    config.set('foo.__proto__.polluted_proto_root', 'Polluted!')
    expect({}).not.toHaveProperty('polluted_proto_root')

    config.set('foo.__proto__.nested.polluted_proto_nested', 'Polluted!')
    expect({}).not.toHaveProperty('nested')
    expect({}).not.toHaveProperty('nested.polluted_proto_nested')

    config.set('this.this.constructor.prototype.polluted_constructor_prototype_root', 'Polluted!')
    expect({}).not.toHaveProperty('polluted_constructor_prototype_root')

    config.set('this.this.constructor.prototype.nested.polluted_constructor_prototype_nested', 'Polluted!')
    expect({}).not.toHaveProperty('nested')
    expect({}).not.toHaveProperty('nested.polluted_constructor_prototype_nested')

    config.set('foo.this.constructor.prototype.polluted_constructor_prototype_root', 'Polluted!')
    expect({}).not.toHaveProperty('polluted_constructor_prototype_root')

    config.set('foo.this.constructor.prototype.nested.polluted_constructor_prototype_nested', 'Polluted!')
    expect({}).not.toHaveProperty('nested')
    expect({}).not.toHaveProperty('nested.polluted_constructor_prototype_nested')