Skip to content

Commit

Permalink
tools: add avoid-prototype-pollution lint rule
Browse files Browse the repository at this point in the history 
PR-URL: #43308
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
@aduh95 @danielleadams
aduh95 authored and danielleadams committed Jun 12, 2022
1 parent 3d645ee commit 3891051
Show file tree
Hide file tree
Showing 4 changed files with 245 additions and 5 deletions.
1 change: 1 addition & 0 deletions lib/.eslintrc.yaml
View file
Expand Up @@ -156,6 +156,7 @@ rules:
- name: SubtleCrypto
message: Use `const { SubtleCrypto } = require('internal/crypto/webcrypto');` instead of the global.
# Custom rules in tools/eslint-rules
node-core/avoid-prototype-pollution: error
node-core/lowercase-name-for-primitive: error
node-core/non-ascii-character: error
node-core/no-array-destructuring: error
Expand Down
10 changes: 5 additions & 5 deletions lib/internal/per_context/primordials.js
View file
Expand Up @@ -78,7 +78,7 @@ function copyPropsRenamed(src, dest, prefix) {
copyAccessor(dest, prefix, newKey, desc);
} else {
const name = `${prefix}${newKey}`;
ReflectDefineProperty(dest, name, desc);
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
if (varargsMethods.includes(name)) {
ReflectDefineProperty(dest, `${name}Apply`, {
__proto__: null,
Expand All @@ -105,7 +105,7 @@ function copyPropsRenamedBound(src, dest, prefix) {
}

const name = `${prefix}${newKey}`;
ReflectDefineProperty(dest, name, desc);
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
if (varargsMethods.includes(name)) {
ReflectDefineProperty(dest, `${name}Apply`, {
__proto__: null,
Expand All @@ -129,7 +129,7 @@ function copyPrototype(src, dest, prefix) {
}

const name = `${prefix}${newKey}`;
ReflectDefineProperty(dest, name, desc);
ReflectDefineProperty(dest, name, { __proto__: null, ...desc });
if (varargsMethods.includes(name)) {
ReflectDefineProperty(dest, `${name}Apply`, {
__proto__: null,
Expand Down Expand Up @@ -312,7 +312,7 @@ const copyProps = (src, dest) => {
ReflectDefineProperty(
dest,
key,
ReflectGetOwnPropertyDescriptor(src, key));
{ __proto__: null, ...ReflectGetOwnPropertyDescriptor(src, key) });
}
});
};
Expand Down Expand Up @@ -340,7 +340,7 @@ const makeSafe = (unsafe, safe) => {
return new SafeIterator(this);
};
}
ReflectDefineProperty(safe.prototype, key, desc);
ReflectDefineProperty(safe.prototype, key, { __proto__: null, ...desc });
}
});
} else {
Expand Down
147 changes: 147 additions & 0 deletions test/parallel/test-eslint-avoid-prototype-pollution.js
View file
@@ -0,0 +1,147 @@
'use strict';

const common = require('../common');
if ((!common.hasCrypto) || (!common.hasIntl)) {
common.skip('ESLint tests require crypto and Intl');
}

common.skipIfEslintMissing();

const RuleTester = require('../../tools/node_modules/eslint').RuleTester;
const rule = require('../../tools/eslint-rules/avoid-prototype-pollution');

new RuleTester({
parserOptions: { ecmaVersion: 2022 },
})
.run('property-descriptor-no-prototype-pollution', rule, {
valid: [
'ObjectDefineProperties({}, {})',
'ObjectCreate(null, {})',
'ObjectDefineProperties({}, { key })',
'ObjectCreate(null, { key })',
'ObjectDefineProperties({}, { ...spread })',
'ObjectCreate(null, { ...spread })',
'ObjectDefineProperties({}, { key: valueDescriptor })',
'ObjectCreate(null, { key: valueDescriptor })',
'ObjectDefineProperties({}, { key: { ...{}, __proto__: null } })',
'ObjectCreate(null, { key: { ...{}, __proto__: null } })',
'ObjectDefineProperties({}, { key: { __proto__: null } })',
'ObjectCreate(null, { key: { __proto__: null } })',
'ObjectDefineProperties({}, { key: { __proto__: null, enumerable: true } })',
'ObjectCreate(null, { key: { __proto__: null, enumerable: true } })',
'ObjectDefineProperties({}, { key: { "__proto__": null } })',
'ObjectCreate(null, { key: { "__proto__": null } })',
'ObjectDefineProperties({}, { key: { \'__proto__\': null } })',
'ObjectCreate(null, { key: { \'__proto__\': null } })',
'ObjectDefineProperty({}, "key", ObjectCreate(null))',
'ReflectDefineProperty({}, "key", ObjectCreate(null))',
'ObjectDefineProperty({}, "key", valueDescriptor)',
'ReflectDefineProperty({}, "key", valueDescriptor)',
'ObjectDefineProperty({}, "key", { __proto__: null })',
'ReflectDefineProperty({}, "key", { __proto__: null })',
'ObjectDefineProperty({}, "key", { __proto__: null, enumerable: true })',
'ReflectDefineProperty({}, "key", { __proto__: null, enumerable: true })',
'ObjectDefineProperty({}, "key", { "__proto__": null })',
'ReflectDefineProperty({}, "key", { "__proto__": null })',
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
],
invalid: [
{
code: 'ObjectDefineProperties({}, ObjectGetOwnPropertyDescriptors({}))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ObjectCreate(null, ObjectGetOwnPropertyDescriptors({}))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ObjectDefineProperties({}, { key: {} })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectCreate(null, { key: {} })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperties({}, { key: { [void 0]: { ...{ __proto__: null } } } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectCreate(null, { key: { [void 0]: { ...{ __proto__: null } } } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperties({}, { key: { __proto__: Object.prototype } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectCreate(null, { key: { __proto__: Object.prototype } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperties({}, { key: { [`__proto__`]: null } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectCreate(null, { key: { [`__proto__`]: null } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperties({}, { key: { enumerable: true } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectCreate(null, { key: { enumerable: true } })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperty({}, "key", {})',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ReflectDefineProperty({}, "key", {})',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperty({}, "key", ObjectGetOwnPropertyDescriptor({}, "key"))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ReflectDefineProperty({}, "key", ObjectGetOwnPropertyDescriptor({}, "key"))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ObjectDefineProperty({}, "key", ReflectGetOwnPropertyDescriptor({}, "key"))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ReflectDefineProperty({}, "key", ReflectGetOwnPropertyDescriptor({}, "key"))',
errors: [{ message: /prototype pollution/ }],
},
{
code: 'ObjectDefineProperty({}, "key", { __proto__: Object.prototype })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ReflectDefineProperty({}, "key", { __proto__: Object.prototype })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperty({}, "key", { [`__proto__`]: null })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ReflectDefineProperty({}, "key", { [`__proto__`]: null })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ObjectDefineProperty({}, "key", { enumerable: true })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'ReflectDefineProperty({}, "key", { enumerable: true })',
errors: [{ message: /null-prototype/ }],
},
]
});
92 changes: 92 additions & 0 deletions tools/eslint-rules/avoid-prototype-pollution.js
View file
@@ -0,0 +1,92 @@
'use strict';

function checkProperties(context, node) {
if (
node.type === 'CallExpression' &&
node.callee.name === 'ObjectGetOwnPropertyDescriptors'
) {
context.report({
node,
message:
'Property descriptors inherits from the Object prototype, therefore are subject to prototype pollution',
});
}
if (node.type !== 'ObjectExpression') return;
for (const { key, value } of node.properties) {
if (
key != null && value != null &&
!(key.type === 'Identifier' && key.name === '__proto__') &&
!(key.type === 'Literal' && key.value === '__proto__')
) {
checkPropertyDescriptor(context, value);
}
}
}

function checkPropertyDescriptor(context, node) {
if (
node.type === 'CallExpression' &&
(node.callee.name === 'ObjectGetOwnPropertyDescriptor' ||
node.callee.name === 'ReflectGetOwnPropertyDescriptor')
) {
context.report({
node,
message:
'Property descriptors inherits from the Object prototype, therefore are subject to prototype pollution',
suggest: [{
desc: 'Wrap the property descriptor in a null-prototype object',
fix(fixer) {
return [
fixer.insertTextBefore(node, '{ __proto__: null,...'),
fixer.insertTextAfter(node, ' }'),
];
},
}],
});
}
if (node.type !== 'ObjectExpression') return;

for (const { key, value } of node.properties) {
if (
key != null && value != null &&
((key.type === 'Identifier' && key.name === '__proto__') ||
(key.type === 'Literal' && key.value === '__proto__')) &&
value.type === 'Literal' && value.value === null
) {
return true;
}
}

context.report({
node,
message: 'Must use null-prototype object for property descriptors',
});
}

const CallExpression = 'ExpressionStatement[expression.type="CallExpression"]';
module.exports = {
meta: { hasSuggestions: true },
create(context) {
return {
[`${CallExpression}[expression.callee.name=${/^(Object|Reflect)DefinePropert(ies|y)$/}]`](
node
) {
switch (node.expression.callee.name) {
case 'ObjectDefineProperties':
checkProperties(context, node.expression.arguments[1]);
break;
case 'ReflectDefineProperty':
case 'ObjectDefineProperty':
checkPropertyDescriptor(context, node.expression.arguments[2]);
break;
default:
throw new Error('Unreachable');
}
},

[`${CallExpression}[expression.callee.name="ObjectCreate"][expression.arguments.length=2]`](node) {
checkProperties(context, node.expression.arguments[1]);
},
};
},
};

0 comments on commit 3891051

Please sign in to comment.