Skip to content

Commit f95ef06

Browse files
RafaelGSSrichardlau
RafaelGSS
authored and
richardlau
committed
lib: makeRequireFunction patch when experimental policy
Signed-off-by: RafaelGSS <[email protected]> Backport-PR-URL: nodejs-private/node-private#372 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: Michael Dawson <[email protected]>
1 parent b02d895 commit f95ef06

9 files changed

+38
-18
lines changed

‎lib/internal/modules/cjs/helpers.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,8 @@ function makeRequireFunction(mod, redirects) {
104104
};
105105
} else {
106106
require = function require(path) {
107-
return mod[require_private_symbol](mod, path);
107+
// When no policy manifest, the original prototype.require is sustained
108+
return mod.require(path);
108109
};
109110
}
110111

‎lib/internal/modules/cjs/loader.js‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -224,10 +224,9 @@ function Module(id = '', parent) {
224224
if (policy?.manifest) {
225225
const moduleURL = pathToFileURL(id);
226226
redirects = policy.manifest.getDependencyMapper(moduleURL);
227+
// TODO(rafaelgss): remove the necessity of this branch
228+
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
227229
}
228-
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
229-
// Loads a module at the given file path. Returns that module's
230-
// `exports` property.
231230
this[require_private_symbol] = internalRequire;
232231
}
233232

@@ -1080,6 +1079,23 @@ Module.prototype.load = function(filename) {
10801079
esmLoader.cjsCache.set(this, exports);
10811080
};
10821081

1082+
// Loads a module at the given file path. Returns that module's
1083+
// `exports` property.
1084+
// Note: when using the experimental policy mechanism this function is overridden
1085+
Module.prototype.require = function(id) {
1086+
validateString(id, 'id');
1087+
if (id === '') {
1088+
throw new ERR_INVALID_ARG_VALUE('id', id,
1089+
'must be a non-empty string');
1090+
}
1091+
requireDepth++;
1092+
try {
1093+
return Module._load(id, this, /* isMain */ false);
1094+
} finally {
1095+
requireDepth--;
1096+
}
1097+
};
1098+
10831099
// Resolved path to process.argv[1] will be lazily placed here
10841100
// (needed for setting breakpoint when called with --inspect-brk)
10851101
let resolvedArgv;
@@ -1127,10 +1143,12 @@ function wrapSafe(filename, content, cjsModuleInstance) {
11271143
// Returns exception, if any.
11281144
Module.prototype._compile = function(content, filename) {
11291145
let moduleURL;
1130-
if (policy?.manifest) {
1146+
let redirects;
1147+
const manifest = policy?.manifest;
1148+
if (manifest) {
11311149
moduleURL = pathToFileURL(filename);
1132-
policy.manifest.getDependencyMapper(moduleURL);
1133-
policy.manifest.assertIntegrity(moduleURL, content);
1150+
redirects = manifest.getDependencyMapper(moduleURL);
1151+
manifest.assertIntegrity(moduleURL, content);
11341152
}
11351153

11361154
maybeCacheSourceMap(filename, content, this);
@@ -1160,17 +1178,18 @@ Module.prototype._compile = function(content, filename) {
11601178
}
11611179
}
11621180
const dirname = path.dirname(filename);
1181+
const require = makeRequireFunction(this, redirects);
11631182
let result;
11641183
const exports = this.exports;
11651184
const thisValue = exports;
11661185
const module = this;
11671186
if (requireDepth === 0) statCache = new SafeMap();
11681187
if (inspectorWrapper) {
11691188
result = inspectorWrapper(compiledWrapper, thisValue, exports,
1170-
module.require, module, filename, dirname);
1189+
require, module, filename, dirname);
11711190
} else {
11721191
result = ReflectApply(compiledWrapper, thisValue,
1173-
[exports, module.require, module, filename, dirname]);
1192+
[exports, require, module, filename, dirname]);
11741193
}
11751194
hasLoadedAnyUserCJSModule = true;
11761195
if (requireDepth === 0) statCache = null;

‎test/message/source_map_disabled_by_api.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ Error: an error!
88
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
99
at Module.load (node:internal/modules/cjs/loader:*)
1010
at Function.Module._load (node:internal/modules/cjs/loader:*)
11-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
11+
at Module.require (node:internal/modules/cjs/loader:*)
1212
*enclosing-call-site.js:16
1313
throw new Error('an error!')
1414
^
@@ -23,4 +23,4 @@ Error: an error!
2323
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
2424
at Module.load (node:internal/modules/cjs/loader:*)
2525
at Function.Module._load (node:internal/modules/cjs/loader:*)
26-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
26+
at Module.require (node:internal/modules/cjs/loader:*)

‎test/message/source_map_enabled_by_api.out‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ Error: an error!
1212
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
1313
at Module.load (node:internal/modules/cjs/loader:*)
1414
at Function.Module._load (node:internal/modules/cjs/loader:*)
15-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
15+
at Module.require (node:internal/modules/cjs/loader:*)
1616
*enclosing-call-site-min.js:1
1717
var functionA=function(){functionB()};function functionB(){functionC()}var functionC=function(){functionD()},functionD=function(){if(0<Math.random())throw Error("an error!");},thrower=functionA;try{functionA()}catch(a){throw a;};
1818
^
@@ -27,4 +27,4 @@ Error: an error!
2727
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
2828
at Module.load (node:internal/modules/cjs/loader:*)
2929
at Function.Module._load (node:internal/modules/cjs/loader:*)
30-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
30+
at Module.require (node:internal/modules/cjs/loader:*)

‎test/message/source_map_enclosing_function.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,4 @@ Error: an error!
1212
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
1313
at Module.load (node:internal/modules/cjs/loader:*)
1414
at Function.Module._load (node:internal/modules/cjs/loader:*)
15-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
15+
at Module.require (node:internal/modules/cjs/loader:*)

‎test/message/source_map_reference_error_tabs.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ ReferenceError: alert is not defined
99
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*
1010
at Module.load (node:internal/modules/cjs/loader:*
1111
at Function.Module._load (node:internal/modules/cjs/loader:*
12-
at Module.internalRequire (node:internal/modules/cjs/loader:*
12+
at Module.require (node:internal/modules/cjs/loader:*
1313
at require (node:internal/modules/cjs/helpers:*
1414
at Object.<anonymous> (*source_map_reference_error_tabs.js:*
1515
at Module._compile (node:internal/modules/cjs/loader:*

‎test/message/source_map_throw_catch.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Error: an exception
99
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
1010
at Module.load (node:internal/modules/cjs/loader:*)
1111
at Function.Module._load (node:internal/modules/cjs/loader:*)
12-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
12+
at Module.require (node:internal/modules/cjs/loader:*)
1313
at require (node:internal/modules/cjs/helpers:*)
1414
at Object.<anonymous> (*source_map_throw_catch.js:6:3)
1515
at Module._compile (node:internal/modules/cjs/loader:*)

‎test/message/source_map_throw_first_tick.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Error: an exception
99
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*)
1010
at Module.load (node:internal/modules/cjs/loader:*)
1111
at Function.Module._load (node:internal/modules/cjs/loader:*)
12-
at Module.internalRequire (node:internal/modules/cjs/loader:*)
12+
at Module.require (node:internal/modules/cjs/loader:*)
1313
at require (node:internal/modules/cjs/helpers:*)
1414
at Object.<anonymous> (*source_map_throw_first_tick.js:5:1)
1515
at Module._compile (node:internal/modules/cjs/loader:*)

‎test/message/source_map_throw_icu.out‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ Error: an error
99
at Object.Module._extensions..js (node:internal/modules/cjs/loader:*
1010
at Module.load (node:internal/modules/cjs/loader:*
1111
at Function.Module._load (node:internal/modules/cjs/loader:*
12-
at Module.internalRequire (node:internal/modules/cjs/loader:*
12+
at Module.require (node:internal/modules/cjs/loader:*
1313
at require (node:internal/modules/cjs/helpers:*
1414
at Object.<anonymous> (*source_map_throw_icu.js:*
1515
at Module._compile (node:internal/modules/cjs/loader:*

0 commit comments

Comments
 (0)