From 383d7d58a1d55cf8216b82141aaad821e4d813a1 Mon Sep 17 00:00:00 2001
From: Aleksandr Kovalko <gistrec@gmail.com>
Date: Wed, 20 May 2026 14:10:54 +0200
Subject: [PATCH] fix: prevent prototype pollution in mergeDeep

PR #712 added a `__proto__` / `constructor` guard to `MergeDeep.compareDeep`
(see lib/mergeDeep.js:95-97). The sibling function `MergeDeep.mergeDeep`,
which performs the same kind of recursive object merging and is the one
that actually applies the parsed YAML / JSON config into the running
target, did not receive the matching guard.

Two paths still allowed an attacker-controlled `__proto__` key in a parsed
config object to redirect the merged target's prototype chain:

1. `mergeEmptyTarget` (line 312) used `Object.assign({}, source)`. When
   `source` has `__proto__` as an own enumerable property (which is what
   `JSON.parse` and `js-yaml` produce), `Object.assign` invokes the
   `__proto__` setter on the new target and rewrites its prototype.
   Replaced with a manual copy that skips both reserved keys.

2. `mergeDeep`'s main `for..in` loop (line 327) iterated every key on
   `source`, including `__proto__` and `constructor`, before recursing
   into `this.mergeDeep(target[key], source[key])`. Added the same guard
   as `compareDeep`.

Added a regression test in `test/unit/lib/mergeDeep.test.js` that calls
`mergeDeep({}, JSON.parse('{"__proto__":{"polluted":"yes"}, ...}'))` and
asserts that `Object.prototype` and the returned object are both clean.

`npm run test:unit` passes (117 tests, +1 from this change).
---
 lib/mergeDeep.js                | 16 +++++++++++++++-
 test/unit/lib/mergeDeep.test.js | 27 +++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js
index ab278e5c2..0e098d515 100644
--- a/lib/mergeDeep.js
+++ b/lib/mergeDeep.js
@@ -309,7 +309,17 @@ class MergeDeep {
     if (Array.isArray(source)) {
       target = Array.isArray(target) ? target.concat(source) : source
     } else {
-      target = Object.assign({}, source)
+      // Manual copy instead of Object.assign({}, source) so that a source
+      // with `__proto__` or `constructor` as an own enumerable property
+      // (e.g. from YAML or JSON.parse) cannot redirect the new target's
+      // prototype chain via the setter invoked by Object.assign.
+      target = {}
+      for (const key in source) {
+        if (key === '__proto__' || key === 'constructor') {
+          continue
+        }
+        target[key] = source[key]
+      }
     }
     return target
   }
@@ -325,6 +335,10 @@ class MergeDeep {
       }
 
       for (const key in source) {
+        // Skip prototype pollution vectors
+        if (key === '__proto__' || key === 'constructor') {
+          continue
+        }
         // If the attribute of the object or the element of the array is not a simple primitive
         if (this.isObjectNotArray(source[key]) || Array.isArray(source[key])) {
           // Deep merge Array so that if the same element is there in source and target,
diff --git a/test/unit/lib/mergeDeep.test.js b/test/unit/lib/mergeDeep.test.js
index dd8bd60ba..81aa9e54d 100644
--- a/test/unit/lib/mergeDeep.test.js
+++ b/test/unit/lib/mergeDeep.test.js
@@ -1882,4 +1882,31 @@ branches:
     expect(same.additions).toEqual({})
     expect(same.modifications).toEqual({})
   })
+
+  it('mergeDeep does not allow prototype pollution', () => {
+    const mockGitHub = jest.fn().mockReturnValue({ request: () => {} })
+    const mergeDeep = new MergeDeep(log, mockGitHub, [])
+
+    // js-yaml parses `__proto__:` as an own property and existing call sites
+    // (e.g. configManager.loadGlobalSettingsYaml -> Settings.syncAll -> mergeDeep)
+    // pass the parsed config straight into mergeDeep. Using JSON.parse here
+    // produces the same own-property shape without depending on js-yaml.
+    const malicious = JSON.parse('{"__proto__":{"polluted":"yes"},"constructor":{"poisoned":"yes"}}')
+
+    // Sanity: a fresh object must not already have these props.
+    expect({}.polluted).toBeUndefined()
+    expect({}.poisoned).toBeUndefined()
+
+    const merged = mergeDeep.mergeDeep({}, malicious)
+
+    // Object.prototype must remain clean.
+    expect({}.polluted).toBeUndefined()
+    expect({}.poisoned).toBeUndefined()
+    expect(Object.prototype.polluted).toBeUndefined()
+    expect(Object.prototype.poisoned).toBeUndefined()
+
+    // The merged result also should not carry the pollution payload.
+    expect(merged.polluted).toBeUndefined()
+    expect(merged.poisoned).toBeUndefined()
+  })
 })