GeezerVison is showing github.com — tools: refactor `avoid-prototype-pollution` lint rule · nodejs/node@254358c
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/net.js +6Lines changed: 6 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -29,10 +29,16 @@ const IPv6Reg = new RegExp('^(' + | 29 | 29 | ')(%[0-9a-zA-Z-.:]{1,})?$'); | 30 | 30 |
| 31 | 31 | function isIPv4(s) { | | 32 | + // TODO(aduh95): Replace RegExpPrototypeTest with RegExpPrototypeExec when it | | 33 | + // no longer creates a perf regression in the dns benchmark. | | 34 | + // eslint-disable-next-line node-core/avoid-prototype-pollution | 32 | 35 | return RegExpPrototypeTest(IPv4Reg, s); | 33 | 36 | } | 34 | 37 |
| 35 | 38 | function isIPv6(s) { | | 39 | + // TODO(aduh95): Replace RegExpPrototypeTest with RegExpPrototypeExec when it | | 40 | + // no longer creates a perf regression in the dns benchmark. | | 41 | + // eslint-disable-next-line node-core/avoid-prototype-pollution | 36 | 42 | return RegExpPrototypeTest(IPv6Reg, s); | 37 | 43 | } | 38 | 44 |
| |
Collapse file Copy file name to clipboardExpand all lines: test/parallel/test-eslint-avoid-prototype-pollution.js +23Lines changed: 23 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -45,6 +45,9 @@ new RuleTester({ | 45 | 45 | 'ReflectDefineProperty({}, "key", { "__proto__": null })', | 46 | 46 | 'ObjectDefineProperty({}, "key", { \'__proto__\': null })', | 47 | 47 | 'ReflectDefineProperty({}, "key", { \'__proto__\': null })', | | 48 | + 'StringPrototypeReplace("some string", "some string", "some replacement")', | | 49 | + 'StringPrototypeReplaceAll("some string", "some string", "some replacement")', | | 50 | + 'StringPrototypeSplit("some string", "some string")', | 48 | 51 | 'new Proxy({}, otherObject)', | 49 | 52 | 'new Proxy({}, someFactory())', | 50 | 53 | 'new Proxy({}, { __proto__: null })', | @@ -167,18 +170,38 @@ new RuleTester({ | 167 | 170 | code: 'StringPrototypeMatch("some string", /some regex/)', | 168 | 171 | errors: [{ message: /looks up the Symbol\.match property/ }], | 169 | 172 | }, | | 173 | + { | | 174 | + code: 'let v = StringPrototypeMatch("some string", /some regex/)', | | 175 | + errors: [{ message: /looks up the Symbol\.match property/ }], | | 176 | + }, | | 177 | + { | | 178 | + code: 'let v = StringPrototypeMatch("some string", new RegExp("some regex"))', | | 179 | + errors: [{ message: /looks up the Symbol\.match property/ }], | | 180 | + }, | 170 | 181 | { | 171 | 182 | code: 'StringPrototypeMatchAll("some string", /some regex/)', | 172 | 183 | errors: [{ message: /looks up the Symbol\.matchAll property/ }], | 173 | 184 | }, | | 185 | + { | | 186 | + code: 'let v = StringPrototypeMatchAll("some string", new RegExp("some regex"))', | | 187 | + errors: [{ message: /looks up the Symbol\.matchAll property/ }], | | 188 | + }, | 174 | 189 | { | 175 | 190 | code: 'StringPrototypeReplace("some string", /some regex/, "some replacement")', | 176 | 191 | errors: [{ message: /looks up the Symbol\.replace property/ }], | 177 | 192 | }, | | 193 | + { | | 194 | + code: 'StringPrototypeReplace("some string", new RegExp("some regex"), "some replacement")', | | 195 | + errors: [{ message: /looks up the Symbol\.replace property/ }], | | 196 | + }, | 178 | 197 | { | 179 | 198 | code: 'StringPrototypeReplaceAll("some string", /some regex/, "some replacement")', | 180 | 199 | errors: [{ message: /looks up the Symbol\.replace property/ }], | 181 | 200 | }, | | 201 | + { | | 202 | + code: 'StringPrototypeReplaceAll("some string", new RegExp("some regex"), "some replacement")', | | 203 | + errors: [{ message: /looks up the Symbol\.replace property/ }], | | 204 | + }, | 182 | 205 | { | 183 | 206 | code: 'StringPrototypeSearch("some string", /some regex/)', | 184 | 207 | errors: [{ message: /looks up the Symbol\.search property/ }], | |
Collapse file Copy file name to clipboardExpand all lines: tools/eslint-rules/avoid-prototype-pollution.js +42-23Lines changed: 42 additions & 23 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 1 | 1 | 'use strict'; | 2 | 2 |
| | 3 | +const CallExpression = (fnName) => `CallExpression[callee.name=${fnName}]`; | | 4 | + | 3 | 5 | function checkProperties(context, node) { | 4 | 6 | if ( | 5 | 7 | node.type === 'CallExpression' && | @@ -64,8 +66,10 @@ function checkPropertyDescriptor(context, node) { | 64 | 66 | } | 65 | 67 |
| 66 | 68 | function createUnsafeStringMethodReport(context, name, lookedUpProperty) { | | 69 | + const lastDotPosition = '$String.prototype.'.length; | | 70 | + const unsafePrimordialName = `StringPrototype${name.charAt(lastDotPosition).toUpperCase()}${name.slice(lastDotPosition + 1, -1)}`; | 67 | 71 | return { | 68 | | - [`${CallExpression}[expression.callee.name=${JSON.stringify(name)}]`](node) { | | 72 | + [CallExpression(unsafePrimordialName)](node) { | 69 | 73 | context.report({ | 70 | 74 | node, | 71 | 75 | message: `${name} looks up the ${lookedUpProperty} property on the first argument`, | @@ -74,31 +78,46 @@ function createUnsafeStringMethodReport(context, name, lookedUpProperty) { | 74 | 78 | }; | 75 | 79 | } | 76 | 80 |
| 77 | | -const CallExpression = 'ExpressionStatement[expression.type="CallExpression"]'; | | 81 | +function createUnsafeStringMethodOnRegexReport(context, name, lookedUpProperty) { | | 82 | + const dotPosition = 'Symbol.'.length; | | 83 | + const safePrimordialName = `RegExpPrototypeSymbol${lookedUpProperty.charAt(dotPosition).toUpperCase()}${lookedUpProperty.slice(dotPosition + 1)}`; | | 84 | + const lastDotPosition = '$String.prototype.'.length; | | 85 | + const unsafePrimordialName = `StringPrototype${name.charAt(lastDotPosition).toUpperCase()}${name.slice(lastDotPosition + 1, -1)}`; | | 86 | + return { | | 87 | + [[ | | 88 | + `${CallExpression(unsafePrimordialName)}[arguments.1.type=Literal][arguments.1.regex]`, | | 89 | + `${CallExpression(unsafePrimordialName)}[arguments.1.type=NewExpression][arguments.1.callee.name=RegExp]`, | | 90 | + ].join(',')](node) { | | 91 | + context.report({ | | 92 | + node, | | 93 | + message: `${name} looks up the ${lookedUpProperty} property of the passed regex, use ${safePrimordialName} directly`, | | 94 | + }); | | 95 | + } | | 96 | + }; | | 97 | +} | | 98 | + | 78 | 99 | module.exports = { | 79 | 100 | meta: { hasSuggestions: true }, | 80 | 101 | create(context) { | 81 | 102 | return { | 82 | | - [`${CallExpression}[expression.callee.name=${/^(Object|Reflect)DefinePropert(ies|y)$/}]`]( | 83 | | - node | 84 | | - ) { | 85 | | - switch (node.expression.callee.name) { | | 103 | + [CallExpression(/^(Object|Reflect)DefinePropert(ies|y)$/)](node) { | | 104 | + switch (node.callee.name) { | 86 | 105 | case 'ObjectDefineProperties': | 87 | | - checkProperties(context, node.expression.arguments[1]); | | 106 | + checkProperties(context, node.arguments[1]); | 88 | 107 | break; | 89 | 108 | case 'ReflectDefineProperty': | 90 | 109 | case 'ObjectDefineProperty': | 91 | | - checkPropertyDescriptor(context, node.expression.arguments[2]); | | 110 | + checkPropertyDescriptor(context, node.arguments[2]); | 92 | 111 | break; | 93 | 112 | default: | 94 | 113 | throw new Error('Unreachable'); | 95 | 114 | } | 96 | 115 | }, | 97 | 116 |
| 98 | | - [`${CallExpression}[expression.callee.name="ObjectCreate"][expression.arguments.length=2]`](node) { | 99 | | - checkProperties(context, node.expression.arguments[1]); | | 117 | + [`${CallExpression('ObjectCreate')}[arguments.length=2]`](node) { | | 118 | + checkProperties(context, node.arguments[1]); | 100 | 119 | }, | 101 | | - [`${CallExpression}[expression.callee.name="RegExpPrototypeTest"]`](node) { | | 120 | + [CallExpression('RegExpPrototypeTest')](node) { | 102 | 121 | context.report({ | 103 | 122 | node, | 104 | 123 | message: '%RegExp.prototype.test% looks up the "exec" property of `this` value', | @@ -116,18 +135,18 @@ module.exports = { | 116 | 135 | }], | 117 | 136 | }); | 118 | 137 | }, | 119 | | - [`${CallExpression}[expression.callee.name=${/^RegExpPrototypeSymbol(Match|MatchAll|Search)$/}]`](node) { | | 138 | + [CallExpression(/^RegExpPrototypeSymbol(Match|MatchAll|Search)$/)](node) { | 120 | 139 | context.report({ | 121 | 140 | node, | 122 | | - message: node.expression.callee.name + ' looks up the "exec" property of `this` value', | | 141 | + message: node.callee.name + ' looks up the "exec" property of `this` value', | 123 | 142 | }); | 124 | 143 | }, | 125 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeMatch', 'Symbol.match'), | 126 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeMatchAll', 'Symbol.matchAll'), | 127 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeReplace', 'Symbol.replace'), | 128 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeReplaceAll', 'Symbol.replace'), | 129 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeSearch', 'Symbol.search'), | 130 | | - ...createUnsafeStringMethodReport(context, 'StringPrototypeSplit', 'Symbol.split'), | | 144 | + ...createUnsafeStringMethodReport(context, '%String.prototype.match%', 'Symbol.match'), | | 145 | + ...createUnsafeStringMethodReport(context, '%String.prototype.matchAll%', 'Symbol.matchAll'), | | 146 | + ...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.replace%', 'Symbol.replace'), | | 147 | + ...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.replaceAll%', 'Symbol.replace'), | | 148 | + ...createUnsafeStringMethodReport(context, '%String.prototype.search%', 'Symbol.search'), | | 149 | + ...createUnsafeStringMethodOnRegexReport(context, '%String.prototype.split%', 'Symbol.split'), | 131 | 150 |
| 132 | 151 | 'NewExpression[callee.name="Proxy"][arguments.1.type="ObjectExpression"]'(node) { | 133 | 152 | for (const { key, value } of node.arguments[1].properties) { | @@ -146,15 +165,15 @@ module.exports = { | 146 | 165 | }); | 147 | 166 | }, | 148 | 167 |
| 149 | | - [`${CallExpression}[expression.callee.name=PromisePrototypeCatch]`](node) { | | 168 | + [CallExpression('PromisePrototypeCatch')](node) { | 150 | 169 | context.report({ | 151 | 170 | node, | 152 | 171 | message: '%Promise.prototype.catch% look up the `then` property of ' + | 153 | 172 | 'the `this` argument, use PromisePrototypeThen instead', | 154 | 173 | }); | 155 | 174 | }, | 156 | 175 |
| 157 | | - [`${CallExpression}[expression.callee.name=PromisePrototypeFinally]`](node) { | | 176 | + [CallExpression('PromisePrototypeFinally')](node) { | 158 | 177 | context.report({ | 159 | 178 | node, | 160 | 179 | message: '%Promise.prototype.finally% look up the `then` property of ' + | @@ -163,10 +182,10 @@ module.exports = { | 163 | 182 | }); | 164 | 183 | }, | 165 | 184 |
| 166 | | - [`${CallExpression}[expression.callee.name=${/^Promise(All(Settled)?|Any|Race)/}]`](node) { | | 185 | + [CallExpression(/^Promise(All(Settled)?|Any|Race)/)](node) { | 167 | 186 | context.report({ | 168 | 187 | node, | 169 | | - message: `Use Safe${node.expression.callee.name} instead of ${node.expression.callee.name}`, | | 188 | + message: `Use Safe${node.callee.name} instead of ${node.callee.name}`, | 170 | 189 | }); | 171 | 190 | }, | 172 | 191 | }; | |
|