From 1ea0ab19c5208f66509e1c43b0d0f21c1fd29b75 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Aur=C3=A8le=20DARCHE?=
 <madarche@users.noreply.github.com>
Date: Sat, 7 May 2022 16:37:39 +0200
Subject: [PATCH] More more complete fix for prototype pollution

vulnerability first addressed in #384
---
 CHANGELOG.md                                  |  8 ++++++
 packages/convict/src/main.js                  |  9 +++---
 .../convict/test/prototype_pollution.test.js  | 28 +++++++++++++++++++
 3 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2c20ff6..60fa7ae 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,14 @@ 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
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
@@ -11,8 +11,8 @@ 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'
@@ -567,8 +567,9 @@ 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}.`)) {
+      for (const forbidden_key_path of FORBIDDEN_KEY_PATHS) {
+        if (k.startsWith(forbidden_key_path) ||
+            k.includes(`.${forbidden_key_path}`)) {
           return 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
@@ -14,6 +14,20 @@ describe('Convict prototype pollution resistance', function() {
     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() {
@@ -26,6 +40,20 @@ describe('Convict prototype pollution resistance', function() {
     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')
   })
 
 })