From 0985ef8bfb2aa290e9bbeaaf352d87cb46eab9e8 Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Sat, 17 Dec 2022 20:15:24 +0100 Subject: [PATCH] tools: add `ArrayPrototypeConcat` to the list of primordials to avoid MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/44445 Reviewed-By: Matteo Collina Reviewed-By: Joyee Cheung Reviewed-By: Rafael Gonzaga Reviewed-By: Tobias Nießen --- lib/internal/bootstrap/node.js | 12 ++++++------ lib/internal/debugger/inspect.js | 7 +++---- lib/internal/main/print_help.js | 1 + lib/internal/modules/cjs/loader.js | 16 ++++++++++++---- lib/internal/modules/esm/resolve.js | 12 ++++++------ lib/internal/perf/observe.js | 6 ++++-- lib/internal/util/inspector.js | 9 ++++----- lib/repl.js | 6 ++++-- .../test-eslint-avoid-prototype-pollution.js | 4 ++++ tools/eslint-rules/avoid-prototype-pollution.js | 8 ++++++++ 10 files changed, 52 insertions(+), 29 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index d3c18419fda0ac..bf5690843c66c4 100644 --- a/lib/internal/bootstrap/node.js +++ b/lib/internal/bootstrap/node.js @@ -56,8 +56,8 @@ setupPrepareStackTrace(); const { Array, - ArrayPrototypeConcat, ArrayPrototypeFill, + ArrayPrototypePushApply, FunctionPrototypeCall, JSONParse, ObjectDefineProperty, @@ -162,11 +162,11 @@ const rawMethods = internalBinding('process_methods'); process.getActiveResourcesInfo = function() { const timerCounts = internalTimers.getTimerCounts(); - return ArrayPrototypeConcat( - rawMethods._getActiveRequestsInfo(), - rawMethods._getActiveHandlesInfo(), - ArrayPrototypeFill(new Array(timerCounts.timeoutCount), 'Timeout'), - ArrayPrototypeFill(new Array(timerCounts.immediateCount), 'Immediate')); + const info = rawMethods._getActiveRequestsInfo(); + ArrayPrototypePushApply(info, rawMethods._getActiveHandlesInfo()); + ArrayPrototypePushApply(info, ArrayPrototypeFill(new Array(timerCounts.timeoutCount), 'Timeout')); + ArrayPrototypePushApply(info, ArrayPrototypeFill(new Array(timerCounts.immediateCount), 'Immediate')); + return info; }; // TODO(joyeecheung): remove these diff --git a/lib/internal/debugger/inspect.js b/lib/internal/debugger/inspect.js index 683feac01e981e..691314605cfd7d 100644 --- a/lib/internal/debugger/inspect.js +++ b/lib/internal/debugger/inspect.js @@ -1,11 +1,11 @@ 'use strict'; const { - ArrayPrototypeConcat, ArrayPrototypeForEach, ArrayPrototypeJoin, ArrayPrototypeMap, ArrayPrototypePop, + ArrayPrototypePushApply, ArrayPrototypeShift, ArrayPrototypeSlice, FunctionPrototypeBind, @@ -82,9 +82,8 @@ const debugRegex = /Debugger listening on ws:\/\/\[?(.+?)\]?:(\d+)\//; async function runScript(script, scriptArgs, inspectHost, inspectPort, childPrint) { await portIsFree(inspectHost, inspectPort); - const args = ArrayPrototypeConcat( - [`--inspect-brk=${inspectPort}`, script], - scriptArgs); + const args = [`--inspect-brk=${inspectPort}`, script]; + ArrayPrototypePushApply(args, scriptArgs); const child = spawn(process.execPath, args); child.stdout.setEncoding('utf8'); child.stderr.setEncoding('utf8'); diff --git a/lib/internal/main/print_help.js b/lib/internal/main/print_help.js index bfef215ace8db5..9a9fbc2d2bd195 100644 --- a/lib/internal/main/print_help.js +++ b/lib/internal/main/print_help.js @@ -31,6 +31,7 @@ for (const key of ObjectKeys(types)) // Environment variables are parsed ad-hoc throughout the code base, // so we gather the documentation here. const { hasIntl, hasSmallICU, hasNodeOptions } = internalBinding('config'); +// eslint-disable-next-line node-core/avoid-prototype-pollution const envVars = new SafeMap(ArrayPrototypeConcat([ ['FORCE_COLOR', { helpText: "when set to 'true', 1, 2, 3, or an empty " + 'string causes NO_COLOR and NODE_DISABLE_COLORS to be ignored.' }], diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index ec725bd84b6bb1..46e58400fb34b5 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -23,7 +23,6 @@ const { ArrayIsArray, - ArrayPrototypeConcat, ArrayPrototypeFilter, ArrayPrototypeIncludes, ArrayPrototypeIndexOf, @@ -667,7 +666,13 @@ Module._findPath = function(request, paths, isMain) { Module._pathCache[cacheKey] = filename; return filename; } - reportModuleNotFoundToWatchMode(basePath, ArrayPrototypeConcat([''], exts)); + + if (exts === undefined) { + exts = ['']; + } else { + ArrayPrototypeUnshift(exts, ''); + } + reportModuleNotFoundToWatchMode(basePath, exts); } return false; @@ -781,9 +786,12 @@ Module._resolveLookupPaths = function(request, parent) { StringPrototypeCharAt(request, 1) !== '/' && (!isWindows || StringPrototypeCharAt(request, 1) !== '\\'))) { - let paths = modulePaths; + let paths; if (parent?.paths?.length) { - paths = ArrayPrototypeConcat(parent.paths, paths); + paths = ArrayPrototypeSlice(modulePaths); + ArrayPrototypeUnshiftApply(paths, parent.paths); + } else { + paths = modulePaths; } debug('looking for %j in %j', request, paths); diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index aa979eec36fe8b..edb74a0395d26b 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -2,8 +2,8 @@ const { ArrayIsArray, - ArrayPrototypeConcat, ArrayPrototypeJoin, + ArrayPrototypePush, ArrayPrototypeShift, JSONStringify, ObjectGetOwnPropertyNames, @@ -957,11 +957,11 @@ function throwIfUnsupportedURLScheme(parsed, experimentalNetworkImports) { ) ) ) { - throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, ArrayPrototypeConcat( - 'file', - 'data', - experimentalNetworkImports ? ['https', 'http'] : [], - )); + const schemes = ['file', 'data']; + if (experimentalNetworkImports) { + ArrayPrototypePush(schemes, 'https', 'http'); + } + throw new ERR_UNSUPPORTED_ESM_URL_SCHEME(parsed, schemes); } } diff --git a/lib/internal/perf/observe.js b/lib/internal/perf/observe.js index 2eba4b6c4e2288..9aab2dfbc13ed3 100644 --- a/lib/internal/perf/observe.js +++ b/lib/internal/perf/observe.js @@ -9,7 +9,6 @@ const { ArrayPrototypePushApply, ArrayPrototypeSlice, ArrayPrototypeSort, - ArrayPrototypeConcat, Error, MathMax, MathMin, @@ -513,7 +512,10 @@ function filterBufferMapByNameAndType(name, type) { // Unrecognized type; return []; } else { - bufferList = ArrayPrototypeConcat(markEntryBuffer, measureEntryBuffer, resourceTimingBuffer); + bufferList = []; + ArrayPrototypePushApply(bufferList, markEntryBuffer); + ArrayPrototypePushApply(bufferList, measureEntryBuffer); + ArrayPrototypePushApply(bufferList, resourceTimingBuffer); } if (name !== undefined) { bufferList = ArrayPrototypeFilter(bufferList, (buffer) => buffer.name === name); diff --git a/lib/internal/util/inspector.js b/lib/internal/util/inspector.js index 3a589423b2d280..6940da82a29bac 100644 --- a/lib/internal/util/inspector.js +++ b/lib/internal/util/inspector.js @@ -1,8 +1,8 @@ 'use strict'; const { - ArrayPrototypeConcat, ArrayPrototypeSome, + ArrayPrototypePushApply, FunctionPrototypeBind, ObjectDefineProperty, ObjectKeys, @@ -69,10 +69,9 @@ function installConsoleExtensions(commandLineApi) { const { makeRequireFunction } = require('internal/modules/helpers'); const consoleAPIModule = new CJSModule(''); const cwd = tryGetCwd(); - consoleAPIModule.paths = ArrayPrototypeConcat( - CJSModule._nodeModulePaths(cwd), - CJSModule.globalPaths - ); + consoleAPIModule.paths = []; + ArrayPrototypePushApply(consoleAPIModule.paths, CJSModule._nodeModulePaths(cwd)); + ArrayPrototypePushApply(consoleAPIModule.paths, CJSModule.globalPaths); commandLineApi.require = makeRequireFunction(consoleAPIModule); } diff --git a/lib/repl.js b/lib/repl.js index 4918ab8ad21bc4..40851e4dcd6fcf 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -43,7 +43,6 @@ 'use strict'; const { - ArrayPrototypeConcat, ArrayPrototypeFilter, ArrayPrototypeFindIndex, ArrayPrototypeForEach, @@ -52,6 +51,7 @@ const { ArrayPrototypeMap, ArrayPrototypePop, ArrayPrototypePush, + ArrayPrototypePushApply, ArrayPrototypeReverse, ArrayPrototypeShift, ArrayPrototypeSlice, @@ -1331,7 +1331,9 @@ function complete(line, callback) { } else if (RegExpPrototypeExec(/^\.\.?\//, completeOn) !== null) { paths = [process.cwd()]; } else { - paths = ArrayPrototypeConcat(module.paths, CJSModule.globalPaths); + paths = []; + ArrayPrototypePushApply(paths, module.paths); + ArrayPrototypePushApply(paths, CJSModule.globalPaths); } ArrayPrototypeForEach(paths, (dir) => { diff --git a/test/parallel/test-eslint-avoid-prototype-pollution.js b/test/parallel/test-eslint-avoid-prototype-pollution.js index 76eee4379b965d..dcc43650dbdc6a 100644 --- a/test/parallel/test-eslint-avoid-prototype-pollution.js +++ b/test/parallel/test-eslint-avoid-prototype-pollution.js @@ -295,5 +295,9 @@ new RuleTester({ code: 'PromiseRace([])', errors: [{ message: /\bSafePromiseRace\b/ }] }, + { + code: 'ArrayPrototypeConcat([])', + errors: [{ message: /\bisConcatSpreadable\b/ }] + }, ] }); diff --git a/tools/eslint-rules/avoid-prototype-pollution.js b/tools/eslint-rules/avoid-prototype-pollution.js index 9d842234abde09..d9ea4e936579d4 100644 --- a/tools/eslint-rules/avoid-prototype-pollution.js +++ b/tools/eslint-rules/avoid-prototype-pollution.js @@ -224,6 +224,14 @@ module.exports = { message: `Use Safe${node.callee.name} instead of ${node.callee.name}`, }); }, + + [CallExpression('ArrayPrototypeConcat')](node) { + context.report({ + node, + message: '%Array.prototype.concat% looks up `@@isConcatSpreadable` ' + + 'which can be subject to prototype pollution', + }); + }, }; }, };