From b67284e867de071938063fba65b739fb1e87050e Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Tue, 30 Aug 2022 00:31:06 +0200 Subject: [PATCH 1/2] tools: add `ArrayPrototypeConcat` to the list of primordials to avoid --- 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 | 8 +++++--- 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, 45 insertions(+), 28 deletions(-) diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js index 61d525ea0280bc..168312456abee7 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, Number, @@ -174,11 +174,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 cddd0f426731d6..edaa1a622f3ca0 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, @@ -85,9 +85,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 36efb48fc574cb..7b8b9605dc3e39 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, @@ -769,9 +768,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 e4ad685a6938e7..2effd59faca931 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, ObjectFreeze, @@ -986,11 +986,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 c7f18ffdb61a33..b3e7ceaabf7fdf 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/cjs/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 6edd0c5b2601bd..ebae2556debc97 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, @@ -1332,7 +1332,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 fdc43c259d9a5b..a5c6d03698858b 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', + }); + }, }; }, }; From d3761c2fb776b594d653136178e3082bd5c492eb Mon Sep 17 00:00:00 2001 From: Antoine du Hamel Date: Thu, 15 Dec 2022 00:24:23 +0100 Subject: [PATCH 2/2] fixup! tools: add `ArrayPrototypeConcat` to the list of primordials to avoid --- lib/internal/modules/cjs/loader.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 7b8b9605dc3e39..46ae2d07edbdc6 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -654,7 +654,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;