diff --git a/lib/mergeDeep.js b/lib/mergeDeep.js index ab278e5c..0e098d51 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 dd8bd60b..81aa9e54 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() + }) })