Skip to content

fix: return real constructor on drafts (fixes lodash.isEmpty crash), keep __proto__ pollution guard #1269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jonatankruszewski wants to merge 1 commit into
base: main
Choose a base branch
from
+96 −116
Open

fix: return real constructor on drafts (fixes lodash.isEmpty crash), keep __proto__ pollution guard #1269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
154 changes: 80 additions & 74 deletions __tests__/base.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
58 changes: 16 additions & 42 deletions src/core/proxy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,37 +103,29 @@ export function createProxyProxy<T extends Objectish>(
return [proxy as any, state]
}

const protoGuardTraps: ProxyHandler<any> = {
get(target, key) {
if (key === "__proto__" || key === "prototype") {
return Object.freeze(Object.create(null))
}
return Reflect.get(target, key)
},
set() {
return true
}
}

/**
* Object drafts
*/
export const objectTraps: ProxyHandler<ProxyState> = {
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_
Expand Down Expand Up @@ -183,14 +175,6 @@ export const objectTraps: ProxyHandler<ProxyState> = {
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) {
Expand All @@ -201,16 +185,6 @@ export const objectTraps: ProxyHandler<ProxyState> = {
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
Expand Down