Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

lib: add lint rule to protect against Object.prototype.then pollution #45061

Merged
merged 1 commit into from Oct 21, 2022
Merged

lib: add lint rule to protect against Object.prototype.then pollution #45061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/internal/crypto/cfrg.js
View file
Open in desktop
Expand Up @@ -188,7 +188,7 @@ async function cfrgGenerateKey(algorithm, extractable, keyUsages) {
privateUsages,
extractable);

return { privateKey, publicKey };
return { __proto__: null, privateKey, publicKey };
}

function cfrgExportKey(key, format) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/ec.js
View file
Open in desktop
Expand Up @@ -146,7 +146,7 @@ async function ecGenerateKey(algorithm, extractable, keyUsages) {
privateUsages,
extractable);

return { publicKey, privateKey };
return { __proto__: null, publicKey, privateKey };
}

function ecExportKey(key, format) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/crypto/rsa.js
View file
Open in desktop
Expand Up @@ -219,7 +219,7 @@ async function rsaKeyGenerate(
privateUsages,
extractable);

return { publicKey, privateKey };
return { __proto__: null, publicKey, privateKey };
}

function rsaExportKey(key, format) {
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/fs/cp/cp.js
View file
Open in desktop
Expand Up @@ -115,7 +115,7 @@ async function checkPaths(src, dest, opts) {
code: 'EINVAL',
});
}
return { srcStat, destStat, skipped: false };
return { __proto__: null, srcStat, destStat, skipped: false };
}

function areIdentical(srcStat, destStat) {
Expand Down
16 changes: 8 additions & 8 deletions lib/internal/fs/promises.js
View file
Open in desktop
Expand Up @@ -580,7 +580,7 @@ async function read(handle, bufferOrParams, offset, length, position) {
length |= 0;

if (length === 0)
return { bytesRead: length, buffer };
return { __proto__: null, bytesRead: length, buffer };

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

return { bytesRead, buffer };
return { __proto__: null, bytesRead, buffer };
}

async function readv(handle, buffers, position) {
Expand All @@ -606,12 +606,12 @@ async function readv(handle, buffers, position) {

const bytesRead = (await binding.readBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
return { bytesRead, buffers };
return { __proto__: null, bytesRead, buffers };
}

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

let offset = offsetOrOptions;
if (isArrayBufferView(buffer)) {
Expand All @@ -636,14 +636,14 @@ async function write(handle, buffer, offsetOrOptions, length, position) {
const bytesWritten =
(await binding.writeBuffer(handle.fd, buffer, offset,
length, position, kUsePromises)) || 0;
return { bytesWritten, buffer };
return { __proto__: null, bytesWritten, buffer };
}

validateStringAfterArrayBufferView(buffer, 'buffer');
validateEncoding(buffer, length);
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
length, kUsePromises)) || 0;
return { bytesWritten, buffer };
return { __proto__: null, bytesWritten, buffer };
}

async function writev(handle, buffers, position) {
Expand All @@ -653,12 +653,12 @@ async function writev(handle, buffers, position) {
position = null;

if (buffers.length === 0) {
return { bytesWritten: 0, buffers };
return { __proto__: null, bytesWritten: 0, buffers };
}

const bytesWritten = (await binding.writeBuffers(handle.fd, buffers, position,
kUsePromises)) || 0;
return { bytesWritten, buffers };
return { __proto__: null, bytesWritten, buffers };
}

async function rename(oldPath, newPath) {
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/modules/esm/load.js
View file
Open in desktop
Expand Up @@ -59,7 +59,7 @@ async function getSource(url, context) {
if (policy?.manifest) {
policy.manifest.assertIntegrity(parsed, source);
}
return { responseURL, source };
return { __proto__: null, responseURL, source };
}


Expand Down Expand Up @@ -93,6 +93,7 @@ async function defaultLoad(url, context) {
}

return {
__proto__: null,
format,
responseURL,
source,
Expand Down
3 changes: 3 additions & 0 deletions lib/internal/modules/esm/loader.js
View file
Open in desktop
Expand Up @@ -378,6 +378,7 @@ class ESMLoader {
const { module } = await job.run();

return {
__proto__: null,
namespace: module.getNamespace(),
};
}
Expand Down Expand Up @@ -664,6 +665,7 @@ class ESMLoader {
}

return {
__proto__: null,
format,
responseURL,
source,
Expand Down Expand Up @@ -880,6 +882,7 @@ class ESMLoader {
}

return {
__proto__: null,
format,
url,
};
Expand Down
2 changes: 1 addition & 1 deletion lib/internal/modules/esm/module_job.js
View file
Open in desktop
Expand Up @@ -215,7 +215,7 @@ class ModuleJob {
}
throw e;
}
return { module: this.module };
return { __proto__: null, module: this.module };
}
}
ObjectSetPrototypeOf(ModuleJob.prototype, null);
Expand Down
7 changes: 4 additions & 3 deletions lib/internal/modules/esm/resolve.js
View file
Open in desktop
Expand Up @@ -977,7 +977,7 @@ async function defaultResolve(specifier, context = {}) {
missing = false;
} else if (destination) {
const href = destination.href;
return { url: href };
return { __proto__: null, url: href };
}
if (missing) {
// Prevent network requests from firing if resolution would be banned.
Expand Down Expand Up @@ -1017,7 +1017,7 @@ async function defaultResolve(specifier, context = {}) {
)
)
) {
return { url: parsed.href };
return { __proto__: null, url: parsed.href };
}
} catch {
// Ignore exception
Expand All @@ -1035,7 +1035,7 @@ async function defaultResolve(specifier, context = {}) {
if (maybeReturn) return maybeReturn;

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

throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports);

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

return {
__proto__: null,
// Do NOT cast `url` to a string: that will work even when there are real
// problems, silencing them
url: url.href,
Expand Down
6 changes: 3 additions & 3 deletions lib/internal/webstreams/readablestream.js
View file
Open in desktop
Expand Up @@ -476,7 +476,7 @@ class ReadableStream {

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

if (reader[kState].stream === undefined) {
Expand All @@ -488,11 +488,11 @@ class ReadableStream {
const result = readableStreamReaderGenericCancel(reader, value);
readableStreamReaderGenericRelease(reader);
await result;
return { done: true, value };
return { done: true, value }; // eslint-disable-line node-core/avoid-prototype-pollution
aduh95 marked this conversation as resolved.
Show resolved Hide resolved
}

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

// TODO(@jasnell): Explore whether an async generator
Expand Down
39 changes: 39 additions & 0 deletions test/parallel/test-eslint-avoid-prototype-pollution.js
View file
Open in desktop
Expand Up @@ -45,6 +45,17 @@ new RuleTester({
'ReflectDefineProperty({}, "key", { "__proto__": null })',
'ObjectDefineProperty({}, "key", { \'__proto__\': null })',
'ReflectDefineProperty({}, "key", { \'__proto__\': null })',
'async function myFn() { return { __proto__: null } }',
'async function myFn() { function myFn() { return {} } return { __proto__: null } }',
'const myFn = async function myFn() { return { __proto__: null } }',
'const myFn = async function () { return { __proto__: null } }',
'const myFn = async () => { return { __proto__: null } }',
'const myFn = async () => ({ __proto__: null })',
'function myFn() { return {} }',
'const myFn = function myFn() { return {} }',
'const myFn = function () { return {} }',
'const myFn = () => { return {} }',
'const myFn = () => ({})',
'StringPrototypeReplace("some string", "some string", "some replacement")',
'StringPrototypeReplaceAll("some string", "some string", "some replacement")',
'StringPrototypeSplit("some string", "some string")',
Expand Down Expand Up @@ -150,6 +161,34 @@ new RuleTester({
code: 'ReflectDefineProperty({}, "key", { enumerable: true })',
errors: [{ message: /null-prototype/ }],
},
{
code: 'async function myFn(){ return {} }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'async function myFn(){ async function someOtherFn() { return { __proto__: null } } return {} }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'async function myFn(){ if (true) { return {} } return { __proto__: null } }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'const myFn = async function myFn(){ return {} }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'const myFn = async function (){ return {} }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'const myFn = async () => { return {} }',
errors: [{ message: /null-prototype/ }],
},
{
code: 'const myFn = async () => ({})',
errors: [{ message: /null-prototype/ }],
},
{
code: 'RegExpPrototypeTest(/some regex/, "some string")',
errors: [{ message: /looks up the "exec" property/ }],
Expand Down
56 changes: 40 additions & 16 deletions tools/eslint-rules/avoid-prototype-pollution.js
View file
Open in desktop
@@ -1,6 +1,7 @@
'use strict';

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

function checkProperties(context, node) {
if (
Expand All @@ -25,6 +26,22 @@ function checkProperties(context, node) {
}
}

function isNullPrototypeObjectExpression(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;
}
}
return false;
}

function checkPropertyDescriptor(context, node) {
if (
node.type === 'CallExpression' &&
Expand All @@ -46,23 +63,12 @@ function checkPropertyDescriptor(context, 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;
}
if (isNullPrototypeObjectExpression(node) === false) {
context.report({
node,
message: 'Must use null-prototype object for property descriptors',
});
}

context.report({
node,
message: 'Must use null-prototype object for property descriptors',
});
}

function createUnsafeStringMethodReport(context, name, lookedUpProperty) {
Expand Down Expand Up @@ -117,6 +123,24 @@ module.exports = {
[`${CallExpression('ObjectCreate')}[arguments.length=2]`](node) {
checkProperties(context, node.arguments[1]);
},

[`:matches(${AnyFunction})[async=true]>BlockStatement ReturnStatement>ObjectExpression, :matches(${AnyFunction})[async=true]>ObjectExpression`](node) {
if (node.parent.type === 'ReturnStatement') {
let { parent } = node;
do {
({ parent } = parent);
} while (!parent.type.includes('Function'));

if (!parent.async) return;
}
if (isNullPrototypeObjectExpression(node) === false) {
context.report({
node,
message: 'Use null-prototype when returning from async function',
});
}
},

[CallExpression('RegExpPrototypeTest')](node) {
context.report({
node,
Expand Down