From 848b9330d6ff1df3687b018ca7487420c29b2e35 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 16 Jul 2026 20:05:42 +0200 Subject: [PATCH] fix: Revert "prevent prototype pollution via constructor.prototype access (CVE-2026-XXXX) (#1259)" This reverts commit 48fc3788609ce46dd789c313768bae4f48b00ec5. This fixes #1266 and #1268, #1265 while the security reported could not find a reasonable way to show the designated fixes was closing a real vulnerability, nor could I think of anything. --- .vscode/settings.json | 5 +-- __tests__/base.js | 92 ------------------------------------------- src/core/proxy.ts | 44 --------------------- 3 files changed, 1 insertion(+), 140 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 7d3eb46c..2dc64a8e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,8 +5,5 @@ "javascript.validate.enable": false, "typescript.tsdk": "node_modules/typescript/lib", "jest.enableInlineErrorMessages": true, - "cSpell.enabled": true, - "chat.tools.terminal.autoApprove": { - "yarn vitest": true - } + "cSpell.enabled": true } diff --git a/__tests__/base.js b/__tests__/base.js index a183f1fb..fc249e5a 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -2953,98 +2953,6 @@ function runBaseTest( expect(result.objConstructed).toEqual(new Object().constructor(1)) }) - it("does not allow prototype pollution via reserved constructor access", () => { - const pollutedKey = "__immer_test_polluted__" - const original = Object.prototype[pollutedKey] - - delete Object.prototype[pollutedKey] - - try { - // The attack should either throw an error or silently fail - // but NOT pollute Object.prototype - let hadError = false - try { - produce({}, draft => { - draft.constructor.prototype[pollutedKey] = true - draft["__proto__"][pollutedKey] = true - }) - } catch (e) { - // Expected: error when trying to set on undefined/frozen objects - hadError = true - } - - // The critical check: Object.prototype must not be polluted - // either because we threw an error OR because the assignment was silently blocked - expect(Object.prototype[pollutedKey]).toBeUndefined() - } finally { - if (original === undefined) { - delete Object.prototype[pollutedKey] - } else { - Object.prototype[pollutedKey] = original - } - } - }) - - it("blocks prototype pollution via stored constructor reference (CVE bypass)", () => { - const pollutedKey = "__immer_test_ref__" - const original = Object.prototype[pollutedKey] - - delete Object.prototype[pollutedKey] - - try { - // Attack 3 from CVE: store constructor reference and mutate - let hadError = false - try { - produce({data: {}}, draft => { - const ctor = draft.data.constructor - ctor.prototype[pollutedKey] = true - }) - } catch (e) { - hadError = true - } - - expect(Object.prototype[pollutedKey]).toBeUndefined() - } finally { - if (original === undefined) { - delete Object.prototype[pollutedKey] - } else { - Object.prototype[pollutedKey] = original - } - } - }) - - it("blocks prototype pollution via Object.assign with malicious payload", () => { - const pollutedKey = "__immer_test_assign__" - const original = Object.prototype[pollutedKey] - - delete Object.prototype[pollutedKey] - - try { - // Simulates the real-world attack scenario where user input is Object.assign'd to draft - const userInput = { - constructor: {prototype: {[pollutedKey]: true}} - } - - let hadError = false - try { - produce({}, draft => { - Object.assign(draft, userInput) - }) - } catch (e) { - hadError = true - } - - // Must NOT pollute via Object.assign path - expect(Object.prototype[pollutedKey]).toBeUndefined() - } finally { - if (original === undefined) { - delete Object.prototype[pollutedKey] - } else { - Object.prototype[pollutedKey] = original - } - } - }) - it("should handle equality correctly - 1", () => { const baseState = { y: 3 / 0, diff --git a/src/core/proxy.ts b/src/core/proxy.ts index 43fe72ee..bd98e276 100644 --- a/src/core/proxy.ts +++ b/src/core/proxy.ts @@ -110,32 +110,6 @@ export const objectTraps: ProxyHandler = { get(state, prop) { if (prop === DRAFT_STATE) return state - // Guard against prototype pollution via constructor and __proto__ - // We allow access but wrap in a proxy that blocks prototype chain traversal - if (prop === "constructor" || prop === "__proto__") { - const source = latest(state) - const value = source[prop] - // Return a proxy that allows calling the constructor but blocks access to prototype - return new Proxy(value || {}, { - get: (target, key) => { - // Block __proto__ and prototype access chains - if (key === "__proto__" || key === "prototype") { - return Object.freeze(Object.create(null)) - } - // Allow normal property access for legitimate use - return Reflect.get(target, key) - }, - set: () => { - // Silently ignore writes to prevent pollution - return true - }, - apply: (target, thisArg, args) => { - // Allow constructor to be called as a function (e.g., draft.arr.constructor(1)) - return Reflect.apply(target as Function, thisArg, args) - } - }) - } - let arrayPlugin = state.scope_.arrayMethodsPlugin_ const isArrayWithStringProp = state.type_ === ArchType.Array && typeof prop === "string" @@ -183,14 +157,6 @@ export const objectTraps: ProxyHandler = { return value }, has(state, prop) { - // Block reserved properties from being detected - if ( - prop === "constructor" || - prop === "__proto__" || - prop === "prototype" - ) { - return false - } return prop in latest(state) }, ownKeys(state) { @@ -201,16 +167,6 @@ export const objectTraps: ProxyHandler = { prop: string /* strictly not, but helps TS */, value ) { - // Guard against prototype pollution - prevent assignment to reserved properties - // that could lead to Object.prototype pollution - if ( - prop === "constructor" || - prop === "__proto__" || - prop === "prototype" - ) { - return true - } - const desc = getDescriptorFromProto(latest(state), prop) if (desc?.set) { // special case: if this write is captured by a setter, we have