Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ac9599a

Browse filesBrowse files
aduh95danielleadams
aduh95
authored and
danielleadams
committed
tools: report unsafe string and regex primordials as lint errors
| The string method | looks up the property | | ----------------------------- | --------------------- | | `String.prototype.match` | `Symbol.match` | | `String.prototype.matchAll` | `Symbol.matchAll` | | `String.prototype.replace` | `Symbol.replace` | | `String.prototype.replaceAll` | `Symbol.replace` | | `String.prototype.search` | `Symbol.search` | | `String.prototype.split` | `Symbol.split` | Functions that lookup the `exec` property on the prototype chain: * `RegExp.prototype[Symbol.match]` * `RegExp.prototype[Symbol.matchAll]` * `RegExp.prototype[Symbol.replace]` * `RegExp.prototype[Symbol.search]` * `RegExp.prototype[Symbol.split]` * `RegExp.prototype.test` `RegExp.prototype[Symbol.replace]` and `RegExp.prototype[Symbol.split]` are still allowed for a lack of a better solution. PR-URL: #43393 Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 0bb84b0 commit ac9599a
Copy full SHA for ac9599a

File tree

Expand file treeCollapse file tree

4 files changed

+98
-17
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

4 files changed

+98
-17
lines changed
Open diff view settings
Collapse file

‎lib/_tls_common.js‎

Copy file name to clipboardExpand all lines: lib/_tls_common.js
+16-16Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const {
2727
ArrayPrototypePush,
2828
JSONParse,
2929
ObjectCreate,
30-
StringPrototypeReplace,
30+
RegExpPrototypeSymbolReplace,
3131
} = primordials;
3232

3333
const {
@@ -134,21 +134,21 @@ function translatePeerCertificate(c) {
134134
c.infoAccess = ObjectCreate(null);
135135

136136
// XXX: More key validation?
137-
StringPrototypeReplace(info, /([^\n:]*):([^\n]*)(?:\n|$)/g,
138-
(all, key, val) => {
139-
if (val.charCodeAt(0) === 0x22) {
140-
// The translatePeerCertificate function is only
141-
// used on internally created legacy certificate
142-
// objects, and any value that contains a quote
143-
// will always be a valid JSON string literal,
144-
// so this should never throw.
145-
val = JSONParse(val);
146-
}
147-
if (key in c.infoAccess)
148-
ArrayPrototypePush(c.infoAccess[key], val);
149-
else
150-
c.infoAccess[key] = [val];
151-
});
137+
RegExpPrototypeSymbolReplace(/([^\n:]*):([^\n]*)(?:\n|$)/g, info,
138+
(all, key, val) => {
139+
if (val.charCodeAt(0) === 0x22) {
140+
// The translatePeerCertificate function is only
141+
// used on internally created legacy certificate
142+
// objects, and any value that contains a quote
143+
// will always be a valid JSON string literal,
144+
// so this should never throw.
145+
val = JSONParse(val);
146+
}
147+
if (key in c.infoAccess)
148+
ArrayPrototypePush(c.infoAccess[key], val);
149+
else
150+
c.infoAccess[key] = [val];
151+
});
152152
}
153153
return c;
154154
}
Collapse file

‎lib/repl.js‎

Copy file name to clipboardExpand all lines: lib/repl.js
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -535,7 +535,7 @@ function REPLServer(prompt,
535535

536536
// This will set the values from `savedRegExMatches` to corresponding
537537
// predefined RegExp properties `RegExp.$1`, `RegExp.$2` ... `RegExp.$9`
538-
RegExpPrototypeTest(regExMatcher,
538+
RegExpPrototypeExec(regExMatcher,
539539
ArrayPrototypeJoin(savedRegExMatches, sep));
540540

541541
let finished = false;
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-eslint-avoid-prototype-pollution.js
+40Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,5 +143,45 @@ new RuleTester({
143143
code: 'ReflectDefineProperty({}, "key", { enumerable: true })',
144144
errors: [{ message: /null-prototype/ }],
145145
},
146+
{
147+
code: 'RegExpPrototypeTest(/some regex/, "some string")',
148+
errors: [{ message: /looks up the "exec" property/ }],
149+
},
150+
{
151+
code: 'RegExpPrototypeSymbolMatch(/some regex/, "some string")',
152+
errors: [{ message: /looks up the "exec" property/ }],
153+
},
154+
{
155+
code: 'RegExpPrototypeSymbolMatchAll(/some regex/, "some string")',
156+
errors: [{ message: /looks up the "exec" property/ }],
157+
},
158+
{
159+
code: 'RegExpPrototypeSymbolSearch(/some regex/, "some string")',
160+
errors: [{ message: /looks up the "exec" property/ }],
161+
},
162+
{
163+
code: 'StringPrototypeMatch("some string", /some regex/)',
164+
errors: [{ message: /looks up the Symbol\.match property/ }],
165+
},
166+
{
167+
code: 'StringPrototypeMatchAll("some string", /some regex/)',
168+
errors: [{ message: /looks up the Symbol\.matchAll property/ }],
169+
},
170+
{
171+
code: 'StringPrototypeReplace("some string", /some regex/, "some replacement")',
172+
errors: [{ message: /looks up the Symbol\.replace property/ }],
173+
},
174+
{
175+
code: 'StringPrototypeReplaceAll("some string", /some regex/, "some replacement")',
176+
errors: [{ message: /looks up the Symbol\.replace property/ }],
177+
},
178+
{
179+
code: 'StringPrototypeSearch("some string", /some regex/)',
180+
errors: [{ message: /looks up the Symbol\.search property/ }],
181+
},
182+
{
183+
code: 'StringPrototypeSplit("some string", /some regex/)',
184+
errors: [{ message: /looks up the Symbol\.split property/ }],
185+
},
146186
]
147187
});
Collapse file

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

Copy file name to clipboardExpand all lines: tools/eslint-rules/avoid-prototype-pollution.js
+41Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,17 @@ function checkPropertyDescriptor(context, node) {
6363
});
6464
}
6565

66+
function createUnsafeStringMethodReport(context, name, lookedUpProperty) {
67+
return {
68+
[`${CallExpression}[expression.callee.name=${JSON.stringify(name)}]`](node) {
69+
context.report({
70+
node,
71+
message: `${name} looks up the ${lookedUpProperty} property on the first argument`,
72+
});
73+
}
74+
};
75+
}
76+
6677
const CallExpression = 'ExpressionStatement[expression.type="CallExpression"]';
6778
module.exports = {
6879
meta: { hasSuggestions: true },
@@ -87,6 +98,36 @@ module.exports = {
8798
[`${CallExpression}[expression.callee.name="ObjectCreate"][expression.arguments.length=2]`](node) {
8899
checkProperties(context, node.expression.arguments[1]);
89100
},
101+
[`${CallExpression}[expression.callee.name="RegExpPrototypeTest"]`](node) {
102+
context.report({
103+
node,
104+
message: '%RegExp.prototype.test% looks up the "exec" property of `this` value',
105+
suggest: [{
106+
desc: 'Use RegexpPrototypeExec instead',
107+
fix(fixer) {
108+
const testRange = { ...node.range };
109+
testRange.start = testRange.start + 'RegexpPrototype'.length;
110+
testRange.end = testRange.start + 'Test'.length;
111+
return [
112+
fixer.replaceTextRange(node.range, 'Exec'),
113+
fixer.insertTextAfter(node, ' !== null'),
114+
];
115+
}
116+
}]
117+
});
118+
},
119+
[`${CallExpression}[expression.callee.name=${/^RegExpPrototypeSymbol(Match|MatchAll|Search)$/}]`](node) {
120+
context.report({
121+
node,
122+
message: node.expression.callee.name + ' looks up the "exec" property of `this` value',
123+
});
124+
},
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'),
90131
};
91132
},
92133
};

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.