From e726c3f5ff3dcf793469910cc7875a323bd0b032 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Wed, 20 Jan 2021 11:34:38 +0000 Subject: [PATCH] Cherry-pick of da2bd4f fix: Fixed security issue #738: prototype pollution possible when applying patches CVE-2020-28477 See: CVE-2020-28477 / SNYK-JS-IMMER-1019369 https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28477 https://snyk.io/vuln/SNYK-JS-IMMER-1019369 --- __tests__/patch.js | 109 +++++++++++++++++++++++++++++++++++++++++ src/plugins/patches.ts | 14 +++++- src/utils/errors.ts | 3 +- 3 files changed, 123 insertions(+), 3 deletions(-) diff --git a/__tests__/patch.js b/__tests__/patch.js index 0e7e34f2..1e5d174a 100644 --- a/__tests__/patch.js +++ b/__tests__/patch.js @@ -11,6 +11,8 @@ enableAllPlugins() jest.setTimeout(1000) +const isProd = process.env.NODE_ENV === "production" + function runPatchTest(base, producer, patches, inversePathes) { let resultProxies, resultEs5 @@ -1048,3 +1050,110 @@ test("#559 patches works in a nested reducer with proxies", () => { expect(reversedSubState).toMatchObject(state.sub) }) + +test("do not allow __proto__ polution - 738", () => { + const obj = {} + + // @ts-ignore + expect(obj.polluted).toBe(undefined) + expect(() => { + applyPatches({}, [ + {op: "add", path: ["__proto__", "polluted"], value: "yes"} + ]) + }).toThrow( + isProd + ? "24" + : "Patching reserved attributes like __proto__, prototype and constructor is not allowed" + ) + // @ts-ignore + expect(obj.polluted).toBe(undefined) +}) + +test("do not allow __proto__ polution using arrays - 738", () => { + const obj = {} + const ar = [] + + // @ts-ignore + expect(obj.polluted).toBe(undefined) + // @ts-ignore + expect(ar.polluted).toBe(undefined) + expect(() => { + applyPatches( + [], + [{op: "add", path: ["__proto__", "polluted"], value: "yes"}] + ) + }).toThrow( + isProd + ? "24" + : "Patching reserved attributes like __proto__, prototype and constructor is not allowed" + ) + // @ts-ignore + expect(obj.polluted).toBe(undefined) + // @ts-ignore + expect(ar.polluted).toBe(undefined) +}) + +test("do not allow prototype polution - 738", () => { + const obj = {} + + // @ts-ignore + expect(obj.polluted).toBe(undefined) + expect(() => { + applyPatches(Object, [ + {op: "add", path: ["prototype", "polluted"], value: "yes"} + ]) + }).toThrow( + isProd + ? "24" + : "Patching reserved attributes like __proto__, prototype and constructor is not allowed" + ) + // @ts-ignore + expect(obj.polluted).toBe(undefined) +}) + +test("do not allow constructor polution - 738", () => { + const obj = {} + + // @ts-ignore + expect(obj.polluted).toBe(undefined) + const t = {} + applyPatches(t, [{op: "replace", path: ["constructor"], value: "yes"}]) + expect(typeof t.constructor).toBe("function") + // @ts-ignore + expect(Object.polluted).toBe(undefined) +}) + +test("do not allow constructor.prototype polution - 738", () => { + const obj = {} + + // @ts-ignore + expect(obj.polluted).toBe(undefined) + expect(() => { + applyPatches({}, [ + {op: "add", path: ["constructor", "prototype", "polluted"], value: "yes"} + ]) + }).toThrow( + isProd + ? "24" + : "Patching reserved attributes like __proto__, prototype and constructor is not allowed" + ) + // @ts-ignore + expect(Object.polluted).toBe(undefined) +}) + +test("maps can store __proto__, prototype and constructor props", () => { + const obj = {} + const map = new Map() + map.set("__proto__", {}) + map.set("constructor", {}) + map.set("prototype", {}) + const newMap = applyPatches(map, [ + {op: "add", path: ["__proto__", "polluted"], value: "yes"}, + {op: "add", path: ["constructor", "polluted"], value: "yes"}, + {op: "add", path: ["prototype", "polluted"], value: "yes"} + ]) + expect(newMap.get("__proto__").polluted).toBe("yes") + expect(newMap.get("constructor").polluted).toBe("yes") + expect(newMap.get("prototype").polluted).toBe("yes") + expect(obj.polluted).toBe(undefined) +}) diff --git a/src/plugins/patches.ts b/src/plugins/patches.ts index fe26cf73..c3d4b0af 100644 --- a/src/plugins/patches.ts +++ b/src/plugins/patches.ts @@ -24,7 +24,8 @@ import { ArchtypeMap, ArchtypeSet, ArchtypeArray, - die + die, + ArchtypeObject } from "../internal" import {isDraft} from "../utils/common" @@ -223,7 +224,16 @@ export function enablePatches() { let base: any = draft for (let i = 0; i < path.length - 1; i++) { - base = get(base, path[i]) + const parentType = getArchtype(base) + const p = path[i] + // See #738, avoid prototype pollution + if ( + (parentType === ArchtypeObject || parentType === ArchtypeArray) && + (p === "__proto__" || p === "constructor") + ) + die(24) + if (typeof base === "function" && p === "prototype") die(24) + base = get(base, p) if (typeof base !== "object") die(15, path.join("/")) } diff --git a/src/utils/errors.ts b/src/utils/errors.ts index dcccd8dd..9338f6a0 100644 --- a/src/utils/errors.ts +++ b/src/utils/errors.ts @@ -32,7 +32,8 @@ const errors = { 19(plugin: string) { return "plugin not loaded: " + plugin }, - 20: "Cannot use proxies if Proxy, Proxy.revocable or Reflect are not available" + 20: "Cannot use proxies if Proxy, Proxy.revocable or Reflect are not available", + 24: "Patching reserved attributes like __proto__, prototype and constructor is not allowed" } as const export function die(error: keyof typeof errors, ...args: any[]): never {