From 9ccf8b9a5f1b85d5b0120bf59e927ac685b979f0 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 11 Jun 2022 14:45:31 +0100 Subject: [PATCH] tools: add `avoid-prototype-pollution` lint rule PR-URL: https://github.com/nodejs/node/pull/43308 Reviewed-By: Rich Trott --- lib/.eslintrc.yaml | 1 + lib/internal/per_context/primordials.js | 10 +- .../test-eslint-avoid-prototype-pollution.js | 147 ++++++++++++++++++ .../eslint-rules/avoid-prototype-pollution.js | 92 +++++++++++ 4 files changed, 245 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-eslint-avoid-prototype-pollution.js create mode 100644 tools/eslint-rules/avoid-prototype-pollution.js diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 6bfc102b817cfe..fae5e85ce174b4 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -160,6 +160,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 diff --git a/lib/internal/per_context/primordials.js b/lib/internal/per_context/primordials.js index 5c8df35522b847..bbaad4ea760767 100644 --- a/lib/internal/per_context/primordials.js +++ b/lib/internal/per_context/primordials.js @@ -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, @@ -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, @@ -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, @@ -312,7 +312,7 @@ const copyProps = (src, dest) => { ReflectDefineProperty( dest, key, - ReflectGetOwnPropertyDescriptor(src, key)); + { __proto__: null, ...ReflectGetOwnPropertyDescriptor(src, key) }); } }); }; @@ -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 { diff --git a/test/parallel/test-eslint-avoid-prototype-pollution.js b/test/parallel/test-eslint-avoid-prototype-pollution.js new file mode 100644 index 00000000000000..047def545e9a90 --- /dev/null +++ b/test/parallel/test-eslint-avoid-prototype-pollution.js @@ -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/ }], + }, + ] + }); diff --git a/tools/eslint-rules/avoid-prototype-pollution.js b/tools/eslint-rules/avoid-prototype-pollution.js new file mode 100644 index 00000000000000..bf6bd1e0a81825 --- /dev/null +++ b/tools/eslint-rules/avoid-prototype-pollution.js @@ -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]); + }, + }; + }, +};