OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2026-01-15 12:15:26 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

tools: refactor avoid-prototype-pollution lint rule

Browse Source 
The lint rule was not catching all occurences of unsafe primordials use,
and was too strict on some methods.

PR-URL: https://github.com/nodejs/node/pull/43476
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
This commit is contained in:
Antoine du Hamel
2022-06-17 19:12:42 +02:00
parent ce639629dd
commit 14000517b4
3 changed files with 71 additions and 23 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
lib/internal/net.js
View File

@@ -29,10 +29,16 @@ const IPv6Reg = new RegExp('^(' +
')(%[0-9a-zA-Z-.:]{1,})?$');
function isIPv4(s) {
// TODO(aduh95): Replace RegExpPrototypeTest with RegExpPrototypeExec when it
// no longer creates a perf regression in the dns benchmark.
// eslint-disable-next-line node-core/avoid-prototype-pollution
return RegExpPrototypeTest(IPv4Reg, s);
}
function isIPv6(s) {
// TODO(aduh95): Replace RegExpPrototypeTest with RegExpPrototypeExec when it
// no longer creates a perf regression in the dns benchmark.
// eslint-disable-next-line node-core/avoid-prototype-pollution
return RegExpPrototypeTest(IPv6Reg, s);
}

23
test/parallel/test-eslint-avoid-prototype-pollution.js
View File

@@ -45,6 +45,9 @@ new RuleTester({
'ReflectDefineProperty({}, "key", { "__proto__": null })',
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
'StringPrototypeReplace("some string", "some string", "some replacement")',
'StringPrototypeReplaceAll("some string", "some string", "some replacement")',
'StringPrototypeSplit("some string", "some string")',
'new Proxy({}, otherObject)',
'new Proxy({}, someFactory())',
'new Proxy({}, { __proto__: null })',
@@ -167,18 +170,38 @@ new RuleTester({
code: 'StringPrototypeMatch("some string", /some regex/)',
errors: [{ message: /looks up the Symbol\.match property/ }],
},
{
code: 'let v = StringPrototypeMatch("some string", /some regex/)',
errors: [{ message: /looks up the Symbol\.match property/ }],
},
{
code: 'let v = StringPrototypeMatch("some string", new RegExp("some regex"))',
errors: [{ message: /looks up the Symbol\.match property/ }],
},
{
code: 'StringPrototypeMatchAll("some string", /some regex/)',
errors: [{ message: /looks up the Symbol\.matchAll property/ }],
},
{
code: 'let v = StringPrototypeMatchAll("some string", new RegExp("some regex"))',
errors: [{ message: /looks up the Symbol\.matchAll property/ }],
},
{
code: 'StringPrototypeReplace("some string", /some regex/, "some replacement")',
errors: [{ message: /looks up the Symbol\.replace property/ }],
},
{
code: 'StringPrototypeReplace("some string", new RegExp("some regex"), "some replacement")',
errors: [{ message: /looks up the Symbol\.replace property/ }],
},
{
code: 'StringPrototypeReplaceAll("some string", /some regex/, "some replacement")',
errors: [{ message: /looks up the Symbol\.replace property/ }],
},
{
code: 'StringPrototypeReplaceAll("some string", new RegExp("some regex"), "some replacement")',
errors: [{ message: /looks up the Symbol\.replace property/ }],
},
{
code: 'StringPrototypeSearch("some string", /some regex/)',
errors: [{ message: /looks up the Symbol\.search property/ }],

65
tools/eslint-rules/avoid-prototype-pollution.js
View File

@@ -1,5 +1,7 @@
'use strict';
const CallExpression = (fnName) => `CallExpression[callee.name=${fnName}]`;
function checkProperties(context, node) {
if (
node.type === 'CallExpression' &&
@@ -64,8 +66,10 @@ function checkPropertyDescriptor(context, node) {
}
function createUnsafeStringMethodReport(context, name, lookedUpProperty) {
const lastDotPosition = '$String.prototype.'.length;
const unsafePrimordialName = `StringPrototype${name.charAt(lastDotPosition).toUpperCase()}${name.slice(lastDotPosition + 1, -1)}`;
return {
[`${CallExpression}[expression.callee.name=${JSON.stringify(name)}]`](node) {
[CallExpression(unsafePrimordialName)](node) {
context.report({
node,
message: `${name} looks up the ${lookedUpProperty} property on the first argument`,
@@ -74,31 +78,46 @@ function createUnsafeStringMethodReport(context, name, lookedUpProperty) {
};
}
const CallExpression = 'ExpressionStatement[expression.type="CallExpression"]';
function createUnsafeStringMethodOnRegexReport(context, name, lookedUpProperty) {
const dotPosition = 'Symbol.'.length;
const safePrimordialName = `RegExpPrototypeSymbol${lookedUpProperty.charAt(dotPosition).toUpperCase()}${lookedUpProperty.slice(dotPosition + 1)}`;
const lastDotPosition = '$String.prototype.'.length;
const unsafePrimordialName = `StringPrototype${name.charAt(lastDotPosition).toUpperCase()}${name.slice(lastDotPosition + 1, -1)}`;
return {
[[
`${CallExpression(unsafePrimordialName)}[arguments.1.type=Literal][arguments.1.regex]`,
`${CallExpression(unsafePrimordialName)}[arguments.1.type=NewExpression][arguments.1.callee.name=RegExp]`,
].join(',')](node) {
context.report({
node,
message: `${name} looks up the ${lookedUpProperty} property of the passed regex, use ${safePrimordialName} directly`,
});
}
};
}
module.exports = {
meta: { hasSuggestions: true },
create(context) {
return {
[`${CallExpression}[expression.callee.name=${/^(Object|Reflect)DefinePropert(ies|y)$/}]`](
node
) {
switch (node.expression.callee.name) {
[CallExpression(/^(Object|Reflect)DefinePropert(ies|y)$/)](node) {
switch (node.callee.name) {
case 'ObjectDefineProperties':
checkProperties(context, node.expression.arguments[1]);
checkProperties(context, node.arguments[1]);
break;
case 'ReflectDefineProperty':
case 'ObjectDefineProperty':
checkPropertyDescriptor(context, node.expression.arguments[2]);
checkPropertyDescriptor(context, node.arguments[2]);
break;
default:
throw new Error('Unreachable');
}
},
[`${CallExpression}[expression.callee.name="ObjectCreate"][expression.arguments.length=2]`](node) {
checkProperties(context, node.expression.arguments[1]);
[`${CallExpression('ObjectCreate')}[arguments.length=2]`](node) {
checkProperties(context, node.arguments[1]);
},
[`${CallExpression}[expression.callee.name="RegExpPrototypeTest"]`](node) {
[CallExpression('RegExpPrototypeTest')](node) {
context.report({
node,
message: '%RegExp.prototype.test% looks up the "exec" property of `this` value',
@@ -116,18 +135,18 @@ module.exports = {
}],
});
},
[`${CallExpression}[expression.callee.name=${/^RegExpPrototypeSymbol(Match|MatchAll|Search)$/}]`](node) {
[CallExpression(/^RegExpPrototypeSymbol(Match|MatchAll|Search)$/)](node) {
context.report({
node,
message: node.expression.callee.name + ' looks up the "exec" property of `this` value',
message: node.callee.name + ' looks up the "exec" property of `this` value',
});
},
...createUnsafeStringMethodReport(context, 'StringPrototypeMatch', 'Symbol.match'),
...createUnsafeStringMethodReport(context, 'StringPrototypeMatchAll', 'Symbol.matchAll'),
...createUnsafeStringMethodReport(context, 'StringPrototypeReplace', 'Symbol.replace'),
...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'),
...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'),
...createUnsafeStringMethodReport(context, 'StringPrototypeSplit', 'Symbol.split'),
...createUnsafeStringMethodReport(context, '%String.prototype.match%', 'Symbol.match'),
...createUnsafeStringMethodReport(context, '%String.prototype.matchAll%', 'Symbol.matchAll'),
...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.replace%', 'Symbol.replace'),
...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.replaceAll%', 'Symbol.replace'),
...createUnsafeStringMethodReport(context, '%String.prototype.search%', 'Symbol.search'),
...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.split%', 'Symbol.split'),
'NewExpression[callee.name="Proxy"][arguments.1.type="ObjectExpression"]'(node) {
for (const { key, value } of node.arguments[1].properties) {
@@ -146,7 +165,7 @@ module.exports = {
});
},
[`${CallExpression}[expression.callee.name=PromisePrototypeCatch]`](node) {
[CallExpression('PromisePrototypeCatch')](node) {
context.report({
node,
message: '%Promise.prototype.catch% look up the `then` property of ' +
@@ -154,7 +173,7 @@ module.exports = {
});
},
[`${CallExpression}[expression.callee.name=PromisePrototypeFinally]`](node) {
[CallExpression('PromisePrototypeFinally')](node) {
context.report({
node,
message: '%Promise.prototype.finally% look up the `then` property of ' +
@@ -163,10 +182,10 @@ module.exports = {
});
},
[`${CallExpression}[expression.callee.name=${/^Promise(All(Settled)?|Any|Race)/}]`](node) {
[CallExpression(/^Promise(All(Settled)?|Any|Race)/)](node) {
context.report({
node,
message: `Use Safe${node.expression.callee.name} instead of ${node.expression.callee.name}`,
message: `Use Safe${node.callee.name} instead of ${node.callee.name}`,
});
},
};