Skip to content

Commit da2bd4f

Browse files
authoredJan 20, 2021
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
1 parent d75de70 commit da2bd4f

File tree

3 files changed

+123
-3
lines changed

3 files changed

+123
-3
lines changed
 

‎__tests__/patch.js

+109
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ enableAllPlugins()
1212

1313
jest.setTimeout(1000)
1414

15+
const isProd = process.env.NODE_ENV === "production"
16+
1517
function runPatchTest(base, producer, patches, inversePathes) {
1618
let resultProxies, resultEs5
1719

@@ -1147,3 +1149,110 @@ test("#676 patching Date objects", () => {
11471149
)
11481150
expect(rebuilt.date).toEqual(new Date("2020-11-10T08:08:08.003Z"))
11491151
})
1152+
1153+
test("do not allow __proto__ polution - 738", () => {
1154+
const obj = {}
1155+
1156+
// @ts-ignore
1157+
expect(obj.polluted).toBe(undefined)
1158+
expect(() => {
1159+
applyPatches({}, [
1160+
{op: "add", path: ["__proto__", "polluted"], value: "yes"}
1161+
])
1162+
}).toThrow(
1163+
isProd
1164+
? "24"
1165+
: "Patching reserved attributes like __proto__, prototype and constructor is not allowed"
1166+
)
1167+
// @ts-ignore
1168+
expect(obj.polluted).toBe(undefined)
1169+
})
1170+
1171+
test("do not allow __proto__ polution using arrays - 738", () => {
1172+
const obj = {}
1173+
const ar = []
1174+
1175+
// @ts-ignore
1176+
expect(obj.polluted).toBe(undefined)
1177+
// @ts-ignore
1178+
expect(ar.polluted).toBe(undefined)
1179+
expect(() => {
1180+
applyPatches(
1181+
[],
1182+
[{op: "add", path: ["__proto__", "polluted"], value: "yes"}]
1183+
)
1184+
}).toThrow(
1185+
isProd
1186+
? "24"
1187+
: "Patching reserved attributes like __proto__, prototype and constructor is not allowed"
1188+
)
1189+
// @ts-ignore
1190+
expect(obj.polluted).toBe(undefined)
1191+
// @ts-ignore
1192+
expect(ar.polluted).toBe(undefined)
1193+
})
1194+
1195+
test("do not allow prototype polution - 738", () => {
1196+
const obj = {}
1197+
1198+
// @ts-ignore
1199+
expect(obj.polluted).toBe(undefined)
1200+
expect(() => {
1201+
applyPatches(Object, [
1202+
{op: "add", path: ["prototype", "polluted"], value: "yes"}
1203+
])
1204+
}).toThrow(
1205+
isProd
1206+
? "24"
1207+
: "Patching reserved attributes like __proto__, prototype and constructor is not allowed"
1208+
)
1209+
// @ts-ignore
1210+
expect(obj.polluted).toBe(undefined)
1211+
})
1212+
1213+
test("do not allow constructor polution - 738", () => {
1214+
const obj = {}
1215+
1216+
// @ts-ignore
1217+
expect(obj.polluted).toBe(undefined)
1218+
const t = {}
1219+
applyPatches(t, [{op: "replace", path: ["constructor"], value: "yes"}])
1220+
expect(typeof t.constructor).toBe("function")
1221+
// @ts-ignore
1222+
expect(Object.polluted).toBe(undefined)
1223+
})
1224+
1225+
test("do not allow constructor.prototype polution - 738", () => {
1226+
const obj = {}
1227+
1228+
// @ts-ignore
1229+
expect(obj.polluted).toBe(undefined)
1230+
expect(() => {
1231+
applyPatches({}, [
1232+
{op: "add", path: ["constructor", "prototype", "polluted"], value: "yes"}
1233+
])
1234+
}).toThrow(
1235+
isProd
1236+
? "24"
1237+
: "Patching reserved attributes like __proto__, prototype and constructor is not allowed"
1238+
)
1239+
// @ts-ignore
1240+
expect(Object.polluted).toBe(undefined)
1241+
})
1242+
1243+
test("maps can store __proto__, prototype and constructor props", () => {
1244+
const obj = {}
1245+
const map = new Map()
1246+
map.set("__proto__", {})
1247+
map.set("constructor", {})
1248+
map.set("prototype", {})
1249+
const newMap = applyPatches(map, [
1250+
{op: "add", path: ["__proto__", "polluted"], value: "yes"},
1251+
{op: "add", path: ["constructor", "polluted"], value: "yes"},
1252+
{op: "add", path: ["prototype", "polluted"], value: "yes"}
1253+
])
1254+
expect(newMap.get("__proto__").polluted).toBe("yes")
1255+
expect(newMap.get("constructor").polluted).toBe("yes")
1256+
expect(newMap.get("prototype").polluted).toBe("yes")
1257+
expect(obj.polluted).toBe(undefined)
1258+
})

‎src/plugins/patches.ts

+12-2
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,8 @@ import {
2626
ArchtypeArray,
2727
die,
2828
isDraft,
29-
isDraftable
29+
isDraftable,
30+
ArchtypeObject
3031
} from "../internal"
3132

3233
export function enablePatches() {
@@ -211,7 +212,16 @@ export function enablePatches() {
211212

212213
let base: any = draft
213214
for (let i = 0; i < path.length - 1; i++) {
214-
base = get(base, path[i])
215+
const parentType = getArchtype(base)
216+
const p = path[i]
217+
// See #738, avoid prototype pollution
218+
if (
219+
(parentType === ArchtypeObject || parentType === ArchtypeArray) &&
220+
(p === "__proto__" || p === "constructor")
221+
)
222+
die(24)
223+
if (typeof base === "function" && p === "prototype") die(24)
224+
base = get(base, p)
215225
if (typeof base !== "object") die(15, path.join("/"))
216226
}
217227

‎src/utils/errors.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ const errors = {
3838
},
3939
23(thing: string) {
4040
return `'original' expects a draft, got: ${thing}`
41-
}
41+
},
42+
24: "Patching reserved attributes like __proto__, prototype and constructor is not allowed"
4243
} as const
4344

4445
export function die(error: keyof typeof errors, ...args: any[]): never {

5 commit comments

Comments
 (5)

SteveRuben commented on Jan 24, 2021

@SteveRuben

up date to correct, right ? or already corrected ?

Befa222 commented on Feb 3, 2021

@Befa222

mweststrate commented on Feb 3, 2021

@mweststrate
CollaboratorAuthor

@Befa222 place random comments on your own repos please.

abbaskiko commented on Feb 14, 2021

@abbaskiko

security alert fix

FarukhSaifi commented on Feb 18, 2021

@FarukhSaifi

Fix Dep

This conversation has been locked and limited to collaborators.