From 42966d7047cf26446e7f0caeefe6e1efb2e7620f Mon Sep 17 00:00:00 2001 From: Jonatan Kruszewski Date: Thu, 9 Jul 2026 14:01:34 +0300 Subject: [PATCH] fix: stop constructor draft access crashing lodash, keep __proto__ pollution guard The 11.1.9 hardening (48fc378) wrapped both constructor and __proto__ reads in a sanitizing Proxy. The __proto__ half cleanly blocks draft.__proto__.x = 1 from reaching Object.prototype and is kept. The constructor half cannot work: constructor resolves to a function whose prototype is a non-configurable, non-writable data property, so a wrapping Proxy that hides it violates a Proxy invariant and throws (TypeError: 'get' on proxy: property 'prototype' ...), crashing ecosystem inspection such as lodash.isEmpty(draft). Mutating draft.constructor.prototype is plain-JS Object.prototype mutation immer is not a vector for and cannot intercept without breaking draft.constructor === Object. Keep the __proto__ wrapper; drop the constructor wrapper and the has/set reserved-key blocks. Replace the added tests with ones that distinguish a clean block from a crash. All 3697 tests pass. --- __tests__/base.js | 154 ++++++++++++++++++++++++---------------------- src/core/proxy.ts | 58 +++++------------ 2 files changed, 96 insertions(+), 116 deletions(-) diff --git a/__tests__/base.js b/__tests__/base.js index a183f1fb..a005f736 100644 --- a/__tests__/base.js +++ b/__tests__/base.js @@ -2953,96 +2953,102 @@ 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("does not break constructor/prototype inspection on drafts", () => { + produce({data: {}}, draft => { + expect(draft.constructor).toBe(Object) + expect("constructor" in draft).toBe(true) + expect(() => draft.constructor.prototype).not.toThrow() + expect(draft.constructor.prototype).toBe(Object.prototype) + const ctor = draft.data.constructor + const proto = + (typeof ctor === "function" && ctor.prototype) || Object.prototype + expect(draft.data === proto).toBe(false) + }) }) - it("blocks prototype pollution via stored constructor reference (CVE bypass)", () => { - const pollutedKey = "__immer_test_ref__" - const original = Object.prototype[pollutedKey] + it("preserves own data keys named like reserved properties", () => { + const next = produce({constructor: "hi", prototype: 1}, draft => { + draft.prototype = 42 + draft.extra = "x" + }) + expect(next).toEqual({constructor: "hi", prototype: 42, extra: "x"}) + }) - delete Object.prototype[pollutedKey] + it("blocks setPrototypeOf / __proto__ assignment on a draft", () => { + expect(() => + produce({}, draft => { + Object.setPrototypeOf(draft, {polluted: true}) + }) + ).toThrowError(/setPrototypeOf/) + }) + it("blocks prototype pollution via draft.__proto__ traversal", () => { + const key = "__immer_proto_pollution__" + delete Object.prototype[key] 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() + produce({}, draft => { + draft.__proto__[key] = true + draft["__proto__"][key] = true + }) + expect(Object.prototype[key]).toBeUndefined() + expect({}[key]).toBeUndefined() } finally { - if (original === undefined) { - delete Object.prototype[pollutedKey] - } else { - Object.prototype[pollutedKey] = original - } + delete Object.prototype[key] } }) - it("blocks prototype pollution via Object.assign with malicious payload", () => { - const pollutedKey = "__immer_test_assign__" - const original = Object.prototype[pollutedKey] - - delete Object.prototype[pollutedKey] + describe("__proto__ guard wrapper (protoGuardTraps)", () => { + it("forwards ordinary reads to the real prototype", () => { + produce({}, draft => { + const guard = draft.__proto__ + expect(typeof guard.hasOwnProperty).toBe("function") + expect(typeof guard.toString).toBe("function") + expect(guard.hasOwnProperty).toBe(Object.prototype.hasOwnProperty) + }) + }) - try { - // Simulates the real-world attack scenario where user input is Object.assign'd to draft - const userInput = { - constructor: {prototype: {[pollutedKey]: true}} - } + it("returns a frozen null-prototype object for prototype-chain reads", () => { + produce({}, draft => { + const guard = draft.__proto__ + for (const key of ["__proto__", "prototype"]) { + const blocked = guard[key] + expect(Object.isFrozen(blocked)).toBe(true) + expect(Object.getPrototypeOf(blocked)).toBe(null) + expect(Object.keys(blocked)).toHaveLength(0) + } + }) + }) - let hadError = false + it("silently ignores writes without throwing", () => { + const key = "__immer_guard_write__" + delete Object.prototype[key] try { produce({}, draft => { - Object.assign(draft, userInput) - }) - } catch (e) { - hadError = true + expect(() => { + draft.__proto__[key] = true + }).not.toThrow() + expect(draft.__proto__[key]).toBeUndefined() + }) + expect(Object.prototype[key]).toBeUndefined() + } finally { + delete Object.prototype[key] } + }) - // 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("guards Array.prototype for array drafts", () => { + const key = "__immer_array_proto__" + delete Array.prototype[key] + try { + produce([1, 2, 3], draft => { + draft.__proto__[key] = true + expect(Object.isFrozen(draft.__proto__.prototype)).toBe(true) + }) + expect(Array.prototype[key]).toBeUndefined() + expect([][key]).toBeUndefined() + } finally { + delete Array.prototype[key] } - } + }) }) it("should handle equality correctly - 1", () => { diff --git a/src/core/proxy.ts b/src/core/proxy.ts index 43fe72ee..8235cbe5 100644 --- a/src/core/proxy.ts +++ b/src/core/proxy.ts @@ -103,6 +103,18 @@ export function createProxyProxy( return [proxy as any, state] } +const protoGuardTraps: ProxyHandler = { + get(target, key) { + if (key === "__proto__" || key === "prototype") { + return Object.freeze(Object.create(null)) + } + return Reflect.get(target, key) + }, + set() { + return true + } +} + /** * Object drafts */ @@ -110,30 +122,10 @@ 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) - } - }) + // Prevent prototype pollution via `draft.__proto__` traversal writes. + if (prop === "__proto__") { + const value = latest(state)[prop] + return new Proxy(value || {}, protoGuardTraps) } let arrayPlugin = state.scope_.arrayMethodsPlugin_ @@ -183,14 +175,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 +185,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