From f7892c16be6507e26c2ae478c261fa3fa2d84f52 Mon Sep 17 00:00:00 2001 From: RafaelGSS Date: Mon, 13 Feb 2023 15:41:30 -0300 Subject: [PATCH] lib: makeRequireFunction patch when experimental policy Signed-off-by: RafaelGSS Backport-PR-URL: https://github.com/nodejs-private/node-private/pull/373 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1747642 CVE-ID: CVE-2023-23918 PR-URL: https://github.com/nodejs-private/node-private/pull/358 Reviewed-by: Bradley Farias Reviewed-by: Michael Dawson --- lib/internal/modules/cjs/helpers.js | 3 +- lib/internal/modules/cjs/loader.js | 35 ++++++++++++++----- test/message/source_map_disabled_by_api.out | 4 +-- test/message/source_map_enabled_by_api.out | 4 +-- .../message/source_map_enclosing_function.out | 2 +- .../source_map_reference_error_tabs.out | 2 +- test/message/source_map_throw_catch.out | 2 +- test/message/source_map_throw_first_tick.out | 2 +- test/message/source_map_throw_icu.out | 2 +- 9 files changed, 38 insertions(+), 18 deletions(-) diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js index b5baf5b59be225..edf712791db676 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -103,7 +103,8 @@ function makeRequireFunction(mod, redirects) { }; } else { require = function require(path) { - return mod[require_private_symbol](mod, path); + // When no policy manifest, the original prototype.require is sustained + return mod.require(path); }; } diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 730e4ebddccabd..a192a812fb7ff3 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -194,10 +194,9 @@ function Module(id = '', parent) { if (policy?.manifest) { const moduleURL = pathToFileURL(id); redirects = policy.manifest.getDependencyMapper(moduleURL); + // TODO(rafaelgss): remove the necessity of this branch + setOwnProperty(this, 'require', makeRequireFunction(this, redirects)); } - setOwnProperty(this, 'require', makeRequireFunction(this, redirects)); - // Loads a module at the given file path. Returns that module's - // `exports` property. this[require_private_symbol] = internalRequire; } @@ -990,6 +989,23 @@ Module.prototype.load = function(filename) { ESMLoader.cjsCache.set(this, exports); }; +// Loads a module at the given file path. Returns that module's +// `exports` property. +// When using the experimental policy mechanism this function is overridden +Module.prototype.require = function(id) { + validateString(id, 'id'); + if (id === '') { + throw new ERR_INVALID_ARG_VALUE('id', id, + 'must be a non-empty string'); + } + requireDepth++; + try { + return Module._load(id, this, /* isMain */ false); + } finally { + requireDepth--; + } +}; + // Resolved path to process.argv[1] will be lazily placed here // (needed for setting breakpoint when called with --inspect-brk) let resolvedArgv; @@ -1050,10 +1066,12 @@ function wrapSafe(filename, content, cjsModuleInstance) { // Returns exception, if any. Module.prototype._compile = function(content, filename) { let moduleURL; - if (policy?.manifest) { + let redirects; + const manifest = policy?.manifest; + if (manifest) { moduleURL = pathToFileURL(filename); - policy.manifest.getDependencyMapper(moduleURL); - policy.manifest.assertIntegrity(moduleURL, content); + redirects = manifest.getDependencyMapper(moduleURL); + manifest.assertIntegrity(moduleURL, content); } maybeCacheSourceMap(filename, content, this); @@ -1083,6 +1101,7 @@ Module.prototype._compile = function(content, filename) { } } const dirname = path.dirname(filename); + const require = makeRequireFunction(this, redirects); let result; const exports = this.exports; const thisValue = exports; @@ -1090,10 +1109,10 @@ Module.prototype._compile = function(content, filename) { if (requireDepth === 0) statCache = new SafeMap(); if (inspectorWrapper) { result = inspectorWrapper(compiledWrapper, thisValue, exports, - module.require, module, filename, dirname); + require, module, filename, dirname); } else { result = ReflectApply(compiledWrapper, thisValue, - [exports, module.require, module, filename, dirname]); + [exports, require, module, filename, dirname]); } hasLoadedAnyUserCJSModule = true; if (requireDepth === 0) statCache = null; diff --git a/test/message/source_map_disabled_by_api.out b/test/message/source_map_disabled_by_api.out index a0c4f6d5cb6bff..46762a0b476f10 100644 --- a/test/message/source_map_disabled_by_api.out +++ b/test/message/source_map_disabled_by_api.out @@ -8,7 +8,7 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.internalRequire (internal/modules/cjs/loader.js:*) + at Module.require (internal/modules/cjs/loader.js:*) *enclosing-call-site.js:16 throw new Error('an error!') ^ @@ -23,4 +23,4 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.internalRequire (internal/modules/cjs/loader.js:*) + at Module.require (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_enabled_by_api.out b/test/message/source_map_enabled_by_api.out index a5d53028d602dd..3f111ee2f7dee8 100644 --- a/test/message/source_map_enabled_by_api.out +++ b/test/message/source_map_enabled_by_api.out @@ -12,7 +12,7 @@ Error: an error! at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.internalRequire (internal/modules/cjs/loader.js:*) + at Module.require (internal/modules/cjs/loader.js:*) *enclosing-call-site-min.js:1 var functionA=function(){functionB()};function functionB(){functionC()}var functionC=function(){functionD()},functionD=function(){if(0 (*source_map_reference_error_tabs.js:* at Module._compile (internal/modules/cjs/loader.js:* diff --git a/test/message/source_map_throw_catch.out b/test/message/source_map_throw_catch.out index a228d5f3eaa75b..b2123e6eebd1b4 100644 --- a/test/message/source_map_throw_catch.out +++ b/test/message/source_map_throw_catch.out @@ -9,7 +9,7 @@ Error: an exception at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.internalRequire (internal/modules/cjs/loader.js:*) + at Module.require (internal/modules/cjs/loader.js:*) at require (internal/modules/cjs/helpers.js:*) at Object. (*source_map_throw_catch.js:6:3) at Module._compile (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_first_tick.out b/test/message/source_map_throw_first_tick.out index b99da0283cdf30..e4a8e9ff25bb3e 100644 --- a/test/message/source_map_throw_first_tick.out +++ b/test/message/source_map_throw_first_tick.out @@ -9,7 +9,7 @@ Error: an exception at Object.Module._extensions..js (internal/modules/cjs/loader.js:*) at Module.load (internal/modules/cjs/loader.js:*) at Function.Module._load (internal/modules/cjs/loader.js:*) - at Module.internalRequire (internal/modules/cjs/loader.js:*) + at Module.require (internal/modules/cjs/loader.js:*) at require (internal/modules/cjs/helpers.js:*) at Object. (*source_map_throw_first_tick.js:5:1) at Module._compile (internal/modules/cjs/loader.js:*) diff --git a/test/message/source_map_throw_icu.out b/test/message/source_map_throw_icu.out index c4e7e32c2b08b0..9ceb20125f7d9a 100644 --- a/test/message/source_map_throw_icu.out +++ b/test/message/source_map_throw_icu.out @@ -9,7 +9,7 @@ Error: an error at Object.Module._extensions..js (internal/modules/cjs/loader.js:* at Module.load (internal/modules/cjs/loader.js:* at Function.Module._load (internal/modules/cjs/loader.js:* - at Module.internalRequire (internal/modules/cjs/loader.js:* + at Module.require (internal/modules/cjs/loader.js:* at require (internal/modules/cjs/helpers.js:* at Object. (*source_map_throw_icu.js:* at Module._compile (internal/modules/cjs/loader.js:*