Skip to content

Commit 9b7db62

Browse files
committed
lib: makeRequireFunction patch when experimental policy
Signed-off-by: RafaelGSS <[email protected]> Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1747642 CVE-ID: CVE-2023-23918 PR-URL: nodejs-private/node-private#358 Reviewed-by: Bradley Farias <[email protected]> Reviewed-by: Matteo Collina <[email protected]>
1 parent af91400 commit 9b7db62

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

lib/internal/modules/cjs/loader.js

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,9 @@ function Module(id = '', parent) {
229229
if (manifest) {
230230
const moduleURL = pathToFileURL(id);
231231
redirects = manifest.getDependencyMapper(moduleURL);
232+
// TODO(rafaelgss): remove the necessity of this branch
233+
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
232234
}
233-
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
234-
// Loads a module at the given file path. Returns that module's
235-
// `exports` property.
236235
this[require_private_symbol] = internalRequire;
237236
}
238237

@@ -1143,6 +1142,23 @@ Module.prototype.load = function(filename) {
11431142
cascadedLoader.cjsCache.set(this, exports);
11441143
};
11451144

1145+
// Loads a module at the given file path. Returns that module's
1146+
// `exports` property.
1147+
// Note: when using the experimental policy mechanism this function is overridden
1148+
Module.prototype.require = function(id) {
1149+
validateString(id, 'id');
1150+
if (id === '') {
1151+
throw new ERR_INVALID_ARG_VALUE('id', id,
1152+
'must be a non-empty string');
1153+
}
1154+
requireDepth++;
1155+
try {
1156+
return Module._load(id, this, /* isMain */ false);
1157+
} finally {
1158+
requireDepth--;
1159+
}
1160+
};
1161+
11461162
// Resolved path to process.argv[1] will be lazily placed here
11471163
// (needed for setting breakpoint when called with --inspect-brk)
11481164
let resolvedArgv;
@@ -1211,10 +1227,11 @@ function wrapSafe(filename, content, cjsModuleInstance) {
12111227
// Returns exception, if any.
12121228
Module.prototype._compile = function(content, filename) {
12131229
let moduleURL;
1230+
let redirects;
12141231
const manifest = policy()?.manifest;
12151232
if (manifest) {
12161233
moduleURL = pathToFileURL(filename);
1217-
manifest.getDependencyMapper(moduleURL);
1234+
redirects = manifest.getDependencyMapper(moduleURL);
12181235
manifest.assertIntegrity(moduleURL, content);
12191236
}
12201237

@@ -1244,17 +1261,18 @@ Module.prototype._compile = function(content, filename) {
12441261
}
12451262
}
12461263
const dirname = path.dirname(filename);
1264+
const require = makeRequireFunction(this, redirects);
12471265
let result;
12481266
const exports = this.exports;
12491267
const thisValue = exports;
12501268
const module = this;
12511269
if (requireDepth === 0) statCache = new SafeMap();
12521270
if (inspectorWrapper) {
12531271
result = inspectorWrapper(compiledWrapper, thisValue, exports,
1254-
module.require, module, filename, dirname);
1272+
require, module, filename, dirname);
12551273
} else {
12561274
result = ReflectApply(compiledWrapper, thisValue,
1257-
[exports, module.require, module, filename, dirname]);
1275+
[exports, require, module, filename, dirname]);
12581276
}
12591277
hasLoadedAnyUserCJSModule = true;
12601278
if (requireDepth === 0) statCache = null;

lib/internal/modules/helpers.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ function makeRequireFunction(mod, redirects) {
115115
};
116116
} else {
117117
require = function require(path) {
118-
return mod[require_private_symbol](mod, path);
118+
// When no policy manifest, the original prototype.require is sustained
119+
return mod.require(path);
119120
};
120121
}
121122

0 commit comments

Comments
 (0)