Skip to content

Commit c7b7f2b

Browse files
aduh95RafaelGSS
aduh95
authored and
RafaelGSS
committed
lib: add lint rule to protect against Object.prototype.then pollution
PR-URL: #45061 Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Moshe Atlow <[email protected]> Reviewed-By: Minwoo Jung <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent 93a34fa commit c7b7f2b

File tree

10 files changed

+98
-33
lines changed

10 files changed

+98
-33
lines changed

‎lib/internal/crypto/cfrg.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ async function cfrgGenerateKey(algorithm, extractable, keyUsages) {
188188
privateUsages,
189189
extractable);
190190

191-
return { privateKey, publicKey };
191+
return { __proto__: null, privateKey, publicKey };
192192
}
193193

194194
function cfrgExportKey(key, format) {

‎lib/internal/crypto/ec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ async function ecGenerateKey(algorithm, extractable, keyUsages) {
146146
privateUsages,
147147
extractable);
148148

149-
return { publicKey, privateKey };
149+
return { __proto__: null, publicKey, privateKey };
150150
}
151151

152152
function ecExportKey(key, format) {

‎lib/internal/crypto/rsa.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ async function rsaKeyGenerate(
219219
privateUsages,
220220
extractable);
221221

222-
return { publicKey, privateKey };
222+
return { __proto__: null, publicKey, privateKey };
223223
}
224224

225225
function rsaExportKey(key, format) {

‎lib/internal/fs/cp/cp.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ async function checkPaths(src, dest, opts) {
115115
code: 'EINVAL',
116116
});
117117
}
118-
return { srcStat, destStat, skipped: false };
118+
return { __proto__: null, srcStat, destStat, skipped: false };
119119
}
120120

121121
function areIdentical(srcStat, destStat) {

‎lib/internal/fs/promises.js

+8-8
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ async function read(handle, bufferOrParams, offset, length, position) {
580580
length |= 0;
581581

582582
if (length === 0)
583-
return { bytesRead: length, buffer };
583+
return { __proto__: null, bytesRead: length, buffer };
584584

585585
if (buffer.byteLength === 0) {
586586
throw new ERR_INVALID_ARG_VALUE('buffer', buffer,
@@ -595,7 +595,7 @@ async function read(handle, bufferOrParams, offset, length, position) {
595595
const bytesRead = (await binding.read(handle.fd, buffer, offset, length,
596596
position, kUsePromises)) || 0;
597597

598-
return { bytesRead, buffer };
598+
return { __proto__: null, bytesRead, buffer };
599599
}
600600

601601
async function readv(handle, buffers, position) {
@@ -606,12 +606,12 @@ async function readv(handle, buffers, position) {
606606

607607
const bytesRead = (await binding.readBuffers(handle.fd, buffers, position,
608608
kUsePromises)) || 0;
609-
return { bytesRead, buffers };
609+
return { __proto__: null, bytesRead, buffers };
610610
}
611611

612612
async function write(handle, buffer, offsetOrOptions, length, position) {
613613
if (buffer?.byteLength === 0)
614-
return { bytesWritten: 0, buffer };
614+
return { __proto__: null, bytesWritten: 0, buffer };
615615

616616
let offset = offsetOrOptions;
617617
if (isArrayBufferView(buffer)) {
@@ -636,14 +636,14 @@ async function write(handle, buffer, offsetOrOptions, length, position) {
636636
const bytesWritten =
637637
(await binding.writeBuffer(handle.fd, buffer, offset,
638638
length, position, kUsePromises)) || 0;
639-
return { bytesWritten, buffer };
639+
return { __proto__: null, bytesWritten, buffer };
640640
}
641641

642642
validateStringAfterArrayBufferView(buffer, 'buffer');
643643
validateEncoding(buffer, length);
644644
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
645645
length, kUsePromises)) || 0;
646-
return { bytesWritten, buffer };
646+
return { __proto__: null, bytesWritten, buffer };
647647
}
648648

649649
async function writev(handle, buffers, position) {
@@ -653,12 +653,12 @@ async function writev(handle, buffers, position) {
653653
position = null;
654654

655655
if (buffers.length === 0) {
656-
return { bytesWritten: 0, buffers };
656+
return { __proto__: null, bytesWritten: 0, buffers };
657657
}
658658

659659
const bytesWritten = (await binding.writeBuffers(handle.fd, buffers, position,
660660
kUsePromises)) || 0;
661-
return { bytesWritten, buffers };
661+
return { __proto__: null, bytesWritten, buffers };
662662
}
663663

664664
async function rename(oldPath, newPath) {

‎lib/internal/modules/esm/loader.js

+1
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,7 @@ class ESMLoader {
378378
const { module } = await job.run();
379379

380380
return {
381+
__proto__: null,
381382
namespace: module.getNamespace(),
382383
};
383384
}

‎lib/internal/modules/esm/resolve.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ async function defaultResolve(specifier, context = {}) {
977977
missing = false;
978978
} else if (destination) {
979979
const href = destination.href;
980-
return { url: href };
980+
return { __proto__: null, url: href };
981981
}
982982
if (missing) {
983983
// Prevent network requests from firing if resolution would be banned.
@@ -1035,7 +1035,7 @@ async function defaultResolve(specifier, context = {}) {
10351035
if (maybeReturn) return maybeReturn;
10361036

10371037
// This must come after checkIfDisallowedImport
1038-
if (parsed && parsed.protocol === 'node:') return { url: specifier };
1038+
if (parsed && parsed.protocol === 'node:') return { __proto__: null, url: specifier };
10391039

10401040
throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports);
10411041

@@ -1087,6 +1087,7 @@ async function defaultResolve(specifier, context = {}) {
10871087
throwIfUnsupportedURLProtocol(url);
10881088

10891089
return {
1090+
__proto__: null,
10901091
// Do NOT cast `url` to a string: that will work even when there are real
10911092
// problems, silencing them
10921093
url: url.href,

‎lib/internal/webstreams/readablestream.js

+3-3
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ class ReadableStream {
476476

477477
async function returnSteps(value) {
478478
if (done)
479-
return { done: true, value };
479+
return { done: true, value }; // eslint-disable-line node-core/avoid-prototype-pollution
480480
done = true;
481481

482482
if (reader[kState].stream === undefined) {
@@ -488,11 +488,11 @@ class ReadableStream {
488488
const result = readableStreamReaderGenericCancel(reader, value);
489489
readableStreamReaderGenericRelease(reader);
490490
await result;
491-
return { done: true, value };
491+
return { done: true, value }; // eslint-disable-line node-core/avoid-prototype-pollution
492492
}
493493

494494
readableStreamReaderGenericRelease(reader);
495-
return { done: true, value };
495+
return { done: true, value }; // eslint-disable-line node-core/avoid-prototype-pollution
496496
}
497497

498498
// TODO(@jasnell): Explore whether an async generator

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

+39
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,17 @@ new RuleTester({
4545
'ReflectDefineProperty({}, "key", { "__proto__": null })',
4646
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
4747
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
48+
'async function myFn() { return { __proto__: null } }',
49+
'async function myFn() { function myFn() { return {} } return { __proto__: null } }',
50+
'const myFn = async function myFn() { return { __proto__: null } }',
51+
'const myFn = async function () { return { __proto__: null } }',
52+
'const myFn = async () => { return { __proto__: null } }',
53+
'const myFn = async () => ({ __proto__: null })',
54+
'function myFn() { return {} }',
55+
'const myFn = function myFn() { return {} }',
56+
'const myFn = function () { return {} }',
57+
'const myFn = () => { return {} }',
58+
'const myFn = () => ({})',
4859
'StringPrototypeReplace("some string", "some string", "some replacement")',
4960
'StringPrototypeReplaceAll("some string", "some string", "some replacement")',
5061
'StringPrototypeSplit("some string", "some string")',
@@ -150,6 +161,34 @@ new RuleTester({
150161
code: 'ReflectDefineProperty({}, "key", { enumerable: true })',
151162
errors: [{ message: /null-prototype/ }],
152163
},
164+
{
165+
code: 'async function myFn(){ return {} }',
166+
errors: [{ message: /null-prototype/ }],
167+
},
168+
{
169+
code: 'async function myFn(){ async function someOtherFn() { return { __proto__: null } } return {} }',
170+
errors: [{ message: /null-prototype/ }],
171+
},
172+
{
173+
code: 'async function myFn(){ if (true) { return {} } return { __proto__: null } }',
174+
errors: [{ message: /null-prototype/ }],
175+
},
176+
{
177+
code: 'const myFn = async function myFn(){ return {} }',
178+
errors: [{ message: /null-prototype/ }],
179+
},
180+
{
181+
code: 'const myFn = async function (){ return {} }',
182+
errors: [{ message: /null-prototype/ }],
183+
},
184+
{
185+
code: 'const myFn = async () => { return {} }',
186+
errors: [{ message: /null-prototype/ }],
187+
},
188+
{
189+
code: 'const myFn = async () => ({})',
190+
errors: [{ message: /null-prototype/ }],
191+
},
153192
{
154193
code: 'RegExpPrototypeTest(/some regex/, "some string")',
155194
errors: [{ message: /looks up the "exec" property/ }],

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

+40-16
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const CallExpression = (fnName) => `CallExpression[callee.name=${fnName}]`;
4+
const AnyFunction = 'FunctionDeclaration, FunctionExpression, ArrowFunctionExpression';
45

56
function checkProperties(context, node) {
67
if (
@@ -25,6 +26,22 @@ function checkProperties(context, node) {
2526
}
2627
}
2728

29+
function isNullPrototypeObjectExpression(node) {
30+
if (node.type !== 'ObjectExpression') return;
31+
32+
for (const { key, value } of node.properties) {
33+
if (
34+
key != null && value != null &&
35+
((key.type === 'Identifier' && key.name === '__proto__') ||
36+
(key.type === 'Literal' && key.value === '__proto__')) &&
37+
value.type === 'Literal' && value.value === null
38+
) {
39+
return true;
40+
}
41+
}
42+
return false;
43+
}
44+
2845
function checkPropertyDescriptor(context, node) {
2946
if (
3047
node.type === 'CallExpression' &&
@@ -46,23 +63,12 @@ function checkPropertyDescriptor(context, node) {
4663
}],
4764
});
4865
}
49-
if (node.type !== 'ObjectExpression') return;
50-
51-
for (const { key, value } of node.properties) {
52-
if (
53-
key != null && value != null &&
54-
((key.type === 'Identifier' && key.name === '__proto__') ||
55-
(key.type === 'Literal' && key.value === '__proto__')) &&
56-
value.type === 'Literal' && value.value === null
57-
) {
58-
return true;
59-
}
66+
if (isNullPrototypeObjectExpression(node) === false) {
67+
context.report({
68+
node,
69+
message: 'Must use null-prototype object for property descriptors',
70+
});
6071
}
61-
62-
context.report({
63-
node,
64-
message: 'Must use null-prototype object for property descriptors',
65-
});
6672
}
6773

6874
function createUnsafeStringMethodReport(context, name, lookedUpProperty) {
@@ -117,6 +123,24 @@ module.exports = {
117123
[`${CallExpression('ObjectCreate')}[arguments.length=2]`](node) {
118124
checkProperties(context, node.arguments[1]);
119125
},
126+
127+
[`:matches(${AnyFunction})[async=true]>BlockStatement ReturnStatement>ObjectExpression, :matches(${AnyFunction})[async=true]>ObjectExpression`](node) {
128+
if (node.parent.type === 'ReturnStatement') {
129+
let { parent } = node;
130+
do {
131+
({ parent } = parent);
132+
} while (!parent.type.includes('Function'));
133+
134+
if (!parent.async) return;
135+
}
136+
if (isNullPrototypeObjectExpression(node) === false) {
137+
context.report({
138+
node,
139+
message: 'Use null-prototype when returning from async function',
140+
});
141+
}
142+
},
143+
120144
[CallExpression('RegExpPrototypeTest')](node) {
121145
context.report({
122146
node,

0 commit comments

Comments
 (0)