From 3b86be087d8f14681a9c889d45da7fe3ad9cd880 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marc-Aur=C3=A8le=20DARCHE?=
 <madarche@users.noreply.github.com>
Date: Sun, 27 Mar 2022 11:42:01 +0200
Subject: [PATCH] More complete fix against prototype pollution

first addressed in https://github.com/mozilla/node-convict/pull/384
---
 packages/convict/src/main.js                  | 24 +++++++++-----
 .../convict/test/prototype_pollution.test.js  | 31 +++++++++++++++++++
 2 files changed, 48 insertions(+), 7 deletions(-)
 create mode 100644 packages/convict/test/prototype_pollution.test.js

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