Skip to content

Commit 93728c6

Browse files
aduh95aduh95
authoredJun 11, 2022
tools: add avoid-prototype-pollution lint rule
PR-URL: nodejs#43308 Reviewed-By: Rich Trott <[email protected]>
1 parent 917fcb2 commit 93728c6

File tree

4 files changed

+245
-5
lines changed

4 files changed

+245
-5
lines changed
 

‎lib/.eslintrc.yaml

+1
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ rules:
160160
- name: SubtleCrypto
161161
message: Use `const { SubtleCrypto } = require('internal/crypto/webcrypto');` instead of the global.
162162
# Custom rules in tools/eslint-rules
163+
node-core/avoid-prototype-pollution: error
163164
node-core/lowercase-name-for-primitive: error
164165
node-core/non-ascii-character: error
165166
node-core/no-array-destructuring: error

‎lib/internal/per_context/primordials.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ function copyPropsRenamed(src, dest, prefix) {
7878
copyAccessor(dest, prefix, newKey, desc);
7979
} else {
8080
const name = `${prefix}${newKey}`;
81-
ReflectDefineProperty(dest, name, desc);
81+
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
8282
if (varargsMethods.includes(name)) {
8383
ReflectDefineProperty(dest, `${name}Apply`, {
8484
__proto__: null,
@@ -105,7 +105,7 @@ function copyPropsRenamedBound(src, dest, prefix) {
105105
}
106106

107107
const name = `${prefix}${newKey}`;
108-
ReflectDefineProperty(dest, name, desc);
108+
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
109109
if (varargsMethods.includes(name)) {
110110
ReflectDefineProperty(dest, `${name}Apply`, {
111111
__proto__: null,
@@ -129,7 +129,7 @@ function copyPrototype(src, dest, prefix) {
129129
}
130130

131131
const name = `${prefix}${newKey}`;
132-
ReflectDefineProperty(dest, name, desc);
132+
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
133133
if (varargsMethods.includes(name)) {
134134
ReflectDefineProperty(dest, `${name}Apply`, {
135135
__proto__: null,
@@ -312,7 +312,7 @@ const copyProps = (src, dest) => {
312312
ReflectDefineProperty(
313313
dest,
314314
key,
315-
ReflectGetOwnPropertyDescriptor(src, key));
315+
{ __proto__: null, ...ReflectGetOwnPropertyDescriptor(src, key) });
316316
}
317317
});
318318
};
@@ -340,7 +340,7 @@ const makeSafe = (unsafe, safe) => {
340340
return new SafeIterator(this);
341341
};
342342
}
343-
ReflectDefineProperty(safe.prototype, key, desc);
343+
ReflectDefineProperty(safe.prototype, key, { __proto__: null, ...desc });
344344
}
345345
});
346346
} else {

‎test/parallel/test-eslint-avoid-prototype-pollution.js

+147
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
if ((!common.hasCrypto) || (!common.hasIntl)) {
5+
common.skip('ESLint tests require crypto and Intl');
6+
}
7+
8+
common.skipIfEslintMissing();
9+
10+
const RuleTester = require('../../tools/node_modules/eslint').RuleTester;
11+
const rule = require('../../tools/eslint-rules/avoid-prototype-pollution');
12+
13+
new RuleTester({
14+
parserOptions: { ecmaVersion: 2022 },
15+
})
16+
.run('property-descriptor-no-prototype-pollution', rule, {
17+
valid: [
18+
'ObjectDefineProperties({}, {})',
19+
'ObjectCreate(null, {})',
20+
'ObjectDefineProperties({}, { key })',
21+
'ObjectCreate(null, { key })',
22+
'ObjectDefineProperties({}, { ...spread })',
23+
'ObjectCreate(null, { ...spread })',
24+
'ObjectDefineProperties({}, { key: valueDescriptor })',
25+
'ObjectCreate(null, { key: valueDescriptor })',
26+
'ObjectDefineProperties({}, { key: { ...{}, __proto__: null } })',
27+
'ObjectCreate(null, { key: { ...{}, __proto__: null } })',
28+
'ObjectDefineProperties({}, { key: { __proto__: null } })',
29+
'ObjectCreate(null, { key: { __proto__: null } })',
30+
'ObjectDefineProperties({}, { key: { __proto__: null, enumerable: true } })',
31+
'ObjectCreate(null, { key: { __proto__: null, enumerable: true } })',
32+
'ObjectDefineProperties({}, { key: { "__proto__": null } })',
33+
'ObjectCreate(null, { key: { "__proto__": null } })',
34+
'ObjectDefineProperties({}, { key: { \'__proto__\': null } })',
35+
'ObjectCreate(null, { key: { \'__proto__\': null } })',
36+
'ObjectDefineProperty({}, "key", ObjectCreate(null))',
37+
'ReflectDefineProperty({}, "key", ObjectCreate(null))',
38+
'ObjectDefineProperty({}, "key", valueDescriptor)',
39+
'ReflectDefineProperty({}, "key", valueDescriptor)',
40+
'ObjectDefineProperty({}, "key", { __proto__: null })',
41+
'ReflectDefineProperty({}, "key", { __proto__: null })',
42+
'ObjectDefineProperty({}, "key", { __proto__: null, enumerable: true })',
43+
'ReflectDefineProperty({}, "key", { __proto__: null, enumerable: true })',
44+
'ObjectDefineProperty({}, "key", { "__proto__": null })',
45+
'ReflectDefineProperty({}, "key", { "__proto__": null })',
46+
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
47+
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
48+
],
49+
invalid: [
50+
{
51+
code: 'ObjectDefineProperties({}, ObjectGetOwnPropertyDescriptors({}))',
52+
errors: [{ message: /prototype pollution/ }],
53+
},
54+
{
55+
code: 'ObjectCreate(null, ObjectGetOwnPropertyDescriptors({}))',
56+
errors: [{ message: /prototype pollution/ }],
57+
},
58+
{
59+
code: 'ObjectDefineProperties({}, { key: {} })',
60+
errors: [{ message: /null-prototype/ }],
61+
},
62+
{
63+
code: 'ObjectCreate(null, { key: {} })',
64+
errors: [{ message: /null-prototype/ }],
65+
},
66+
{
67+
code: 'ObjectDefineProperties({}, { key: { [void 0]: { ...{ __proto__: null } } } })',
68+
errors: [{ message: /null-prototype/ }],
69+
},
70+
{
71+
code: 'ObjectCreate(null, { key: { [void 0]: { ...{ __proto__: null } } } })',
72+
errors: [{ message: /null-prototype/ }],
73+
},
74+
{
75+
code: 'ObjectDefineProperties({}, { key: { __proto__: Object.prototype } })',
76+
errors: [{ message: /null-prototype/ }],
77+
},
78+
{
79+
code: 'ObjectCreate(null, { key: { __proto__: Object.prototype } })',
80+
errors: [{ message: /null-prototype/ }],
81+
},
82+
{
83+
code: 'ObjectDefineProperties({}, { key: { [`__proto__`]: null } })',
84+
errors: [{ message: /null-prototype/ }],
85+
},
86+
{
87+
code: 'ObjectCreate(null, { key: { [`__proto__`]: null } })',
88+
errors: [{ message: /null-prototype/ }],
89+
},
90+
{
91+
code: 'ObjectDefineProperties({}, { key: { enumerable: true } })',
92+
errors: [{ message: /null-prototype/ }],
93+
},
94+
{
95+
code: 'ObjectCreate(null, { key: { enumerable: true } })',
96+
errors: [{ message: /null-prototype/ }],
97+
},
98+
{
99+
code: 'ObjectDefineProperty({}, "key", {})',
100+
errors: [{ message: /null-prototype/ }],
101+
},
102+
{
103+
code: 'ReflectDefineProperty({}, "key", {})',
104+
errors: [{ message: /null-prototype/ }],
105+
},
106+
{
107+
code: 'ObjectDefineProperty({}, "key", ObjectGetOwnPropertyDescriptor({}, "key"))',
108+
errors: [{ message: /prototype pollution/ }],
109+
},
110+
{
111+
code: 'ReflectDefineProperty({}, "key", ObjectGetOwnPropertyDescriptor({}, "key"))',
112+
errors: [{ message: /prototype pollution/ }],
113+
},
114+
{
115+
code: 'ObjectDefineProperty({}, "key", ReflectGetOwnPropertyDescriptor({}, "key"))',
116+
errors: [{ message: /prototype pollution/ }],
117+
},
118+
{
119+
code: 'ReflectDefineProperty({}, "key", ReflectGetOwnPropertyDescriptor({}, "key"))',
120+
errors: [{ message: /prototype pollution/ }],
121+
},
122+
{
123+
code: 'ObjectDefineProperty({}, "key", { __proto__: Object.prototype })',
124+
errors: [{ message: /null-prototype/ }],
125+
},
126+
{
127+
code: 'ReflectDefineProperty({}, "key", { __proto__: Object.prototype })',
128+
errors: [{ message: /null-prototype/ }],
129+
},
130+
{
131+
code: 'ObjectDefineProperty({}, "key", { [`__proto__`]: null })',
132+
errors: [{ message: /null-prototype/ }],
133+
},
134+
{
135+
code: 'ReflectDefineProperty({}, "key", { [`__proto__`]: null })',
136+
errors: [{ message: /null-prototype/ }],
137+
},
138+
{
139+
code: 'ObjectDefineProperty({}, "key", { enumerable: true })',
140+
errors: [{ message: /null-prototype/ }],
141+
},
142+
{
143+
code: 'ReflectDefineProperty({}, "key", { enumerable: true })',
144+
errors: [{ message: /null-prototype/ }],
145+
},
146+
]
147+
});

‎tools/eslint-rules/avoid-prototype-pollution.js

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
'use strict';
2+
3+
function checkProperties(context, node) {
4+
if (
5+
node.type === 'CallExpression' &&
6+
node.callee.name === 'ObjectGetOwnPropertyDescriptors'
7+
) {
8+
context.report({
9+
node,
10+
message:
11+
'Property descriptors inherits from the Object prototype, therefore are subject to prototype pollution',
12+
});
13+
}
14+
if (node.type !== 'ObjectExpression') return;
15+
for (const { key, value } of node.properties) {
16+
if (
17+
key != null && value != null &&
18+
!(key.type === 'Identifier' && key.name === '__proto__') &&
19+
!(key.type === 'Literal' && key.value === '__proto__')
20+
) {
21+
checkPropertyDescriptor(context, value);
22+
}
23+
}
24+
}
25+
26+
function checkPropertyDescriptor(context, node) {
27+
if (
28+
node.type === 'CallExpression' &&
29+
(node.callee.name === 'ObjectGetOwnPropertyDescriptor' ||
30+
node.callee.name === 'ReflectGetOwnPropertyDescriptor')
31+
) {
32+
context.report({
33+
node,
34+
message:
35+
'Property descriptors inherits from the Object prototype, therefore are subject to prototype pollution',
36+
suggest: [{
37+
desc: 'Wrap the property descriptor in a null-prototype object',
38+
fix(fixer) {
39+
return [
40+
fixer.insertTextBefore(node, '{ __proto__: null,...'),
41+
fixer.insertTextAfter(node, ' }'),
42+
];
43+
},
44+
}],
45+
});
46+
}
47+
if (node.type !== 'ObjectExpression') return;
48+
49+
for (const { key, value } of node.properties) {
50+
if (
51+
key != null && value != null &&
52+
((key.type === 'Identifier' && key.name === '__proto__') ||
53+
(key.type === 'Literal' && key.value === '__proto__')) &&
54+
value.type === 'Literal' && value.value === null
55+
) {
56+
return true;
57+
}
58+
}
59+
60+
context.report({
61+
node,
62+
message: 'Must use null-prototype object for property descriptors',
63+
});
64+
}
65+
66+
const CallExpression = 'ExpressionStatement[expression.type="CallExpression"]';
67+
module.exports = {
68+
meta: { hasSuggestions: true },
69+
create(context) {
70+
return {
71+
[`${CallExpression}[expression.callee.name=${/^(Object|Reflect)DefinePropert(ies|y)$/}]`](
72+
node
73+
) {
74+
switch (node.expression.callee.name) {
75+
case 'ObjectDefineProperties':
76+
checkProperties(context, node.expression.arguments[1]);
77+
break;
78+
case 'ReflectDefineProperty':
79+
case 'ObjectDefineProperty':
80+
checkPropertyDescriptor(context, node.expression.arguments[2]);
81+
break;
82+
default:
83+
throw new Error('Unreachable');
84+
}
85+
},
86+
87+
[`${CallExpression}[expression.callee.name="ObjectCreate"][expression.arguments.length=2]`](node) {
88+
checkProperties(context, node.expression.arguments[1]);
89+
},
90+
};
91+
},
92+
};

0 commit comments

Comments
 (0)
Please sign in to comment.