Skip to content

Commit ee8a57b

Browse files
tjcouch-silDiegoAndai
authored andcommitted
[utils] Fix GitHub-reported prototype pollution vulnerability in deepmerge (#41652)
1 parent c890b25 commit ee8a57b

File tree

2 files changed

+58
-10
lines changed

2 files changed

+58
-10
lines changed

packages/mui-utils/src/deepmerge/deepmerge.test.ts

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,59 @@ import deepmerge from './deepmerge';
44

55
describe('deepmerge', () => {
66
// https://snyk.io/blog/after-three-years-of-silence-a-new-jquery-prototype-pollution-vulnerability-emerges-once-again/
7-
it('should not be subject to prototype pollution', () => {
8-
deepmerge({}, JSON.parse('{ "myProperty": "a", "__proto__" : { "isAdmin" : true } }'), {
9-
clone: false,
10-
});
7+
it('should not be subject to prototype pollution via __proto__', () => {
8+
const result = deepmerge(
9+
{},
10+
JSON.parse('{ "myProperty": "a", "__proto__" : { "isAdmin" : true } }'),
11+
{
12+
clone: false,
13+
},
14+
);
15+
16+
// @ts-expect-error __proto__ is not on this object type
17+
// eslint-disable-next-line no-proto
18+
expect(result.__proto__).to.have.property('isAdmin');
19+
expect({}).not.to.have.property('isAdmin');
20+
});
21+
22+
// https://cwe.mitre.org/data/definitions/915.html
23+
it('should not be subject to prototype pollution via constructor', () => {
24+
const result = deepmerge(
25+
{},
26+
JSON.parse('{ "myProperty": "a", "constructor" : { "prototype": { "isAdmin" : true } } }'),
27+
{
28+
clone: true,
29+
},
30+
);
31+
32+
expect(result.constructor.prototype).to.have.property('isAdmin');
33+
expect({}).not.to.have.property('isAdmin');
34+
});
35+
36+
// https://cwe.mitre.org/data/definitions/915.html
37+
it('should not be subject to prototype pollution via prototype', () => {
38+
const result = deepmerge(
39+
{},
40+
JSON.parse('{ "myProperty": "a", "prototype": { "isAdmin" : true } }'),
41+
{
42+
clone: false,
43+
},
44+
);
45+
46+
// @ts-expect-error prototype is not on this object type
47+
expect(result.prototype).to.have.property('isAdmin');
48+
expect({}).not.to.have.property('isAdmin');
49+
});
50+
51+
it('should appropriately copy the fields without prototype pollution', () => {
52+
const result = deepmerge(
53+
{},
54+
JSON.parse('{ "myProperty": "a", "__proto__" : { "isAdmin" : true } }'),
55+
);
1156

57+
// @ts-expect-error __proto__ is not on this object type
58+
// eslint-disable-next-line no-proto
59+
expect(result.__proto__).to.have.property('isAdmin');
1260
expect({}).not.to.have.property('isAdmin');
1361
});
1462

packages/mui-utils/src/deepmerge/deepmerge.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,12 @@ export default function deepmerge<T>(
4141

4242
if (isPlainObject(target) && isPlainObject(source)) {
4343
Object.keys(source).forEach((key) => {
44-
// Avoid prototype pollution
45-
if (key === '__proto__') {
46-
return;
47-
}
48-
49-
if (isPlainObject(source[key]) && key in target && isPlainObject(target[key])) {
44+
if (
45+
isPlainObject(source[key]) &&
46+
// Avoid prototype pollution
47+
Object.prototype.hasOwnProperty.call(target, key) &&
48+
isPlainObject(target[key])
49+
) {
5050
// Since `output` is a clone of `target` and we have narrowed `target` in this block we can cast to the same type.
5151
(output as Record<keyof any, unknown>)[key] = deepmerge(target[key], source[key], options);
5252
} else if (options.clone) {

0 commit comments

Comments
 (0)