From f95ef064f41ddba49b5f9b3c75b57f255aee5297 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/372 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 b546da0828a755..35db5a5d739eb5 100644 --- a/lib/internal/modules/cjs/helpers.js +++ b/lib/internal/modules/cjs/helpers.js @@ -104,7 +104,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 ea7bc8b389e9c3..614ee758d2b5de 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -224,10 +224,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; } @@ -1080,6 +1079,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. +// Note: 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; @@ -1127,10 +1143,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); @@ -1160,6 +1178,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; @@ -1167,10 +1186,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 67e80b28c3f977..d2cca7da5297e3 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 (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) - at Module.internalRequire (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) *enclosing-call-site.js:16 throw new Error('an error!') ^ @@ -23,4 +23,4 @@ Error: an error! at Object.Module._extensions..js (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) - at Module.internalRequire (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) diff --git a/test/message/source_map_enabled_by_api.out b/test/message/source_map_enabled_by_api.out index 48d79885c1579f..525ceccec12e4b 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 (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) - at Module.internalRequire (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) *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 (node:internal/modules/cjs/loader:* diff --git a/test/message/source_map_throw_catch.out b/test/message/source_map_throw_catch.out index eb772c64d21a3e..95bba5eee3e9dc 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 (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) - at Module.internalRequire (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) at require (node:internal/modules/cjs/helpers:*) at Object. (*source_map_throw_catch.js:6:3) at Module._compile (node:internal/modules/cjs/loader:*) diff --git a/test/message/source_map_throw_first_tick.out b/test/message/source_map_throw_first_tick.out index df045f29ffec0b..efa97a1d9f56dd 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 (node:internal/modules/cjs/loader:*) at Module.load (node:internal/modules/cjs/loader:*) at Function.Module._load (node:internal/modules/cjs/loader:*) - at Module.internalRequire (node:internal/modules/cjs/loader:*) + at Module.require (node:internal/modules/cjs/loader:*) at require (node:internal/modules/cjs/helpers:*) at Object. (*source_map_throw_first_tick.js:5:1) at Module._compile (node:internal/modules/cjs/loader:*) diff --git a/test/message/source_map_throw_icu.out b/test/message/source_map_throw_icu.out index 9a667746a79b49..78482d73ddf037 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 (node:internal/modules/cjs/loader:* at Module.load (node:internal/modules/cjs/loader:* at Function.Module._load (node:internal/modules/cjs/loader:* - at Module.internalRequire (node:internal/modules/cjs/loader:* + at Module.require (node:internal/modules/cjs/loader:* at require (node:internal/modules/cjs/helpers:* at Object. (*source_map_throw_icu.js:* at Module._compile (node:internal/modules/cjs/loader:*