From 9b7db62276e4a9c97aedf91daf38bf7b7d23fee4 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 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: Matteo Collina --- lib/internal/modules/cjs/loader.js | 30 ++++++++++++++++++++++++------ lib/internal/modules/helpers.js | 3 ++- 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index de7f37ad133042..041aae3ca49fcc 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -229,10 +229,9 @@ function Module(id = '', parent) { if (manifest) { const moduleURL = pathToFileURL(id); redirects = 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; } @@ -1143,6 +1142,23 @@ Module.prototype.load = function(filename) { cascadedLoader.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; @@ -1211,10 +1227,11 @@ function wrapSafe(filename, content, cjsModuleInstance) { // Returns exception, if any. Module.prototype._compile = function(content, filename) { let moduleURL; + let redirects; const manifest = policy()?.manifest; if (manifest) { moduleURL = pathToFileURL(filename); - manifest.getDependencyMapper(moduleURL); + redirects = manifest.getDependencyMapper(moduleURL); manifest.assertIntegrity(moduleURL, content); } @@ -1244,6 +1261,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; @@ -1251,10 +1269,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/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js index b5cc52401f9ddc..5dc23f13e7cc3a 100644 --- a/lib/internal/modules/helpers.js +++ b/lib/internal/modules/helpers.js @@ -115,7 +115,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); }; }