Skip to content

Commit 16b3313

Browse files
jdeniaujdeniau
authored
Merge commit from fork
Fix Prototype Pollution in mergeDeep, toJS, etc.
2 parents 6f772de + fd2ef49 commit 16b3313

12 files changed

Lines changed: 207 additions & 0 deletions

File tree

‎__tests__/Map.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,4 +590,13 @@ describe('Map', () => {
590590
])
591591
);
592592
});
593+
594+
it('toJS / toObject are not sensible to prototype pollution', () => {
595+
type User = { user: string; admin?: boolean };
596+
597+
// @ts-expect-error -- intentionally setting __proto__ to test prototype pollution
598+
const m = Map<User>({ user: 'alice' }).set('__proto__', { admin: true });
599+
expect(m.toObject().admin).toBeUndefined();
600+
expect(m.toJS().admin).toBeUndefined();
601+
});
593602
});

‎__tests__/functional/set.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,4 +39,31 @@ describe('set', () => {
3939
expect(set(originalObject, 'z', 789)).toEqual({ x: 123, y: 456, z: 789 });
4040
expect(originalObject).toEqual({ x: 123, y: 456 });
4141
});
42+
43+
it('is not sensible to prototype pollution via set on plain object', () => {
44+
type User = { user: string; admin?: boolean };
45+
46+
const obj: User = { user: 'Alice' };
47+
// Setting __proto__ key should not change the returned object's prototype chain
48+
// @ts-expect-error -- intentionally setting __proto__ to test prototype pollution
49+
const result = set(obj, '__proto__', { admin: true });
50+
51+
// The returned copy should NOT have 'admin' accessible via prototype
52+
// @ts-expect-error -- testing prototype pollution
53+
expect(result.admin).toBeUndefined();
54+
});
55+
56+
it('is not sensible to prototype pollution via set with JSON.parse source', () => {
57+
type User = { user: string; admin?: boolean };
58+
59+
// JSON.parse creates __proto__ as an own property
60+
const malicious = JSON.parse(
61+
'{"user":"Eve","__proto__":{"admin":true}}'
62+
) as User;
63+
// set on an object that already carries __proto__ from JSON.parse
64+
const result = set(malicious, 'user', 'Alice');
65+
66+
// The returned copy should NOT have 'admin' accessible via prototype pollution
67+
expect(result.admin).toBeUndefined();
68+
});
4269
});

‎__tests__/functional/update.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,28 @@ describe('update', () => {
3737
});
3838
expect(originalObject).toEqual({ x: 123, y: 456 });
3939
});
40+
41+
it('is not sensible to prototype pollution via update on plain object', () => {
42+
type User = { user: string; admin?: boolean };
43+
44+
const obj: User = { user: 'Alice' };
45+
// @ts-expect-error -- intentionally setting __proto__ to test prototype pollution
46+
const result = update(obj, '__proto__', () => ({
47+
admin: true,
48+
})) as unknown as User;
49+
50+
// The returned copy should NOT have 'admin' accessible via prototype
51+
expect(result.admin).toBeUndefined();
52+
});
53+
54+
it('is not sensible to prototype pollution via update with JSON.parse source', () => {
55+
type User = { user: string; admin?: boolean };
56+
57+
// JSON.parse creates __proto__ as an own property
58+
const malicious = JSON.parse('{"user":"Eve","__proto__":{"admin":true}}');
59+
const result = update(malicious, 'user', () => 'Alice') as User;
60+
61+
// The returned copy (via shallowCopy) should NOT have 'admin' via prototype
62+
expect(result.admin).toBeUndefined();
63+
});
4064
});

‎__tests__/merge.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,4 +349,33 @@ describe('merge', () => {
349349
// merging with an empty record should return the same empty record instance
350350
expect(merge(myEmptyRecord, { a: 4 })).toBe(myEmptyRecord);
351351
});
352+
353+
it('is not sensible to prototype pollution', () => {
354+
type User = { user: string; admin?: boolean };
355+
356+
// Simulates: app merges HTTP request body (JSON) into user profile
357+
const userProfile: User = { user: 'Alice' };
358+
const requestBody = JSON.parse('{"user":"Eve","__proto__":{"admin":true}}');
359+
360+
const r1 = mergeDeep(userProfile, requestBody);
361+
362+
expect(r1.user).toBe('Eve'); // Eve (updated correctly)
363+
expect(r1.admin).toBeUndefined();
364+
365+
const r2 = mergeDeepWith((a, b) => b, userProfile, requestBody);
366+
expect(r2.admin).toBeUndefined();
367+
368+
const r3 = merge(userProfile, requestBody);
369+
expect(r3.admin).toBeUndefined();
370+
371+
const nested = JSON.parse('{"profile":{"__proto__":{"admin":true}}}');
372+
const r6 = mergeDeep<{ profile: { bio: string; admin?: boolean } }>(
373+
{ profile: { bio: 'Hello' } },
374+
nested
375+
);
376+
377+
expect(r6.profile.admin).toBeUndefined();
378+
379+
expect({}.admin).toBeUndefined(); // Confirm NOT global too
380+
});
352381
});

‎__tests__/updateIn.ts‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,4 +458,36 @@ describe('updateIn', () => {
458458
);
459459
});
460460
});
461+
462+
describe('prototype pollution', () => {
463+
it('setIn on plain object with __proto__ key should not pollute returned object', () => {
464+
type User = { profile: { bio: string }; admin?: boolean };
465+
466+
const obj: User = { profile: { bio: 'Hello' } };
467+
const result = setIn(obj, ['__proto__', 'admin'], true);
468+
469+
// The returned object should NOT have 'admin' accessible via prototype
470+
expect(result.admin).toBeUndefined();
471+
});
472+
473+
it('setIn on plain object with nested __proto__ key should not pollute returned object', () => {
474+
type User = { profile: { bio: string }; admin?: boolean };
475+
476+
const obj: User = { profile: { bio: 'Hello' } };
477+
const result = setIn(obj, ['profile', '__proto__', 'admin'], true);
478+
479+
// The nested object should NOT have 'admin' accessible via prototype
480+
expect(result.profile.admin).toBeUndefined();
481+
});
482+
483+
it('updateIn on plain object with __proto__ key should not pollute returned object', () => {
484+
type User = { profile: { bio: string }; admin?: boolean };
485+
486+
const obj: User = { profile: { bio: 'Hello' } };
487+
const result = updateIn(obj, ['__proto__', 'admin'], () => true);
488+
489+
// The returned object should NOT have 'admin' accessible via prototype
490+
expect(result.admin).toBeUndefined();
491+
});
492+
});
461493
});

‎__tests__/utils/shallowCopy.ts‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
import { describe, it, expect } from '@jest/globals';
2+
import shallowCopy from '../../src/utils/shallowCopy';
3+
4+
describe('shallowCopy', () => {
5+
it('copies a plain object', () => {
6+
const obj = { a: 1, b: 2 };
7+
const copy = shallowCopy(obj);
8+
expect(copy).toEqual({ a: 1, b: 2 });
9+
expect(copy).not.toBe(obj);
10+
});
11+
12+
it('copies an array', () => {
13+
const arr = [1, 2, 3];
14+
const copy = shallowCopy(arr);
15+
expect(copy).toEqual([1, 2, 3]);
16+
expect(copy).not.toBe(arr);
17+
});
18+
19+
it('should not propagate __proto__ key from source object', () => {
20+
type User = { user: string; admin?: boolean };
21+
22+
// @ts-expect-error -- testing prototype pollution
23+
delete Object.prototype.admin;
24+
25+
// JSON.parse creates an own property named "__proto__" (not the actual prototype)
26+
const malicious = JSON.parse('{"user":"Eve","__proto__":{"admin":true}}');
27+
28+
const copy = shallowCopy(malicious);
29+
30+
// The copy should NOT have admin on its prototype chain
31+
expect((copy as User).admin).toBeUndefined();
32+
33+
// Global Object prototype should NOT be polluted
34+
expect(({} as User).admin).toBeUndefined();
35+
36+
// @ts-expect-error -- cleanup
37+
delete Object.prototype.admin;
38+
});
39+
40+
it('should not propagate constructor key from source object', () => {
41+
type User = { user: string; admin?: boolean };
42+
43+
const malicious: User = {
44+
user: 'Eve',
45+
// @ts-expect-error -- intentionally setting constructor to test pollution
46+
constructor: { prototype: { admin: true } },
47+
};
48+
49+
const copy = shallowCopy(malicious);
50+
51+
expect((copy as User).admin).toBeUndefined();
52+
53+
// The constructor of a plain new object should still be Object
54+
expect({}.constructor).toBe(Object);
55+
});
56+
});

‎src/functional/merge.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { isIndexed } from '../predicates/isIndexed';
55
import { isKeyed } from '../predicates/isKeyed';
66
import hasOwnProperty from '../utils/hasOwnProperty';
77
import isDataStructure from '../utils/isDataStructure';
8+
import { isProtoKey } from '../utils/protoInjection';
89
import shallowCopy from '../utils/shallowCopy';
910

1011
export function merge(collection, ...sources) {
@@ -52,6 +53,10 @@ export function mergeWithSources(collection, sources, merger) {
5253
merged.push(value);
5354
}
5455
: (value, key) => {
56+
if (isProtoKey(key)) {
57+
return;
58+
}
59+
5560
const hasVal = hasOwnProperty.call(merged, key);
5661
const nextVal =
5762
hasVal && merger ? merger(merged[key], value, key) : value;

‎src/functional/set.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { Collection, Record } from '../../type-definitions/immutable';
22
import { isImmutable } from '../predicates/isImmutable';
33
import hasOwnProperty from '../utils/hasOwnProperty';
44
import isDataStructure from '../utils/isDataStructure';
5+
import { isProtoKey } from '../utils/protoInjection';
56
import shallowCopy from '../utils/shallowCopy';
67

78
/**
@@ -38,6 +39,10 @@ export function set<K, V, C extends Collection<K, V> | { [key: string]: V }>(
3839
key: K | string,
3940
value: V
4041
): C {
42+
if (typeof key === 'string' && isProtoKey(key)) {
43+
return collection;
44+
}
45+
4146
if (!isDataStructure(collection)) {
4247
throw new TypeError(
4348
'Cannot update non-data-structure value: ' + collection

‎src/methods/toObject.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import assertNotInfinite from '../utils/assertNotInfinite';
2+
import { isProtoKey } from '../utils/protoInjection';
23

34
export function toObject() {
45
assertNotInfinite(this.size);
56
const object = {};
67
this.__iterate((v, k) => {
8+
if (isProtoKey(k)) {
9+
return;
10+
}
11+
712
object[k] = v;
813
});
914
return object;

‎src/toJS.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Seq } from './Seq';
44
import { isCollection } from './predicates/isCollection';
55
import { isKeyed } from './predicates/isKeyed';
66
import isDataStructure from './utils/isDataStructure';
7+
import { isProtoKey } from './utils/protoInjection';
78

89
export function toJS(
910
value: Collection | Record
@@ -26,6 +27,10 @@ export function toJS(
2627
const result: { [key: string]: unknown } = {};
2728
// @ts-expect-error `__iterate` exists on all Keyed collections but method is not defined in the type
2829
value.__iterate((v, k) => {
30+
if (isProtoKey(k)) {
31+
return;
32+
}
33+
2934
result[k] = toJS(v);
3035
});
3136
return result;

0 commit comments

Comments
 (0)