Skip to content

Commit f7892c1

Browse files
RafaelGSSrichardlau
RafaelGSS
authored and
richardlau
committed
lib: makeRequireFunction patch when experimental policy
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> Backport-PR-URL: nodejs-private/node-private#373 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 <bradley.meck@gmail.com> Reviewed-by: Michael Dawson <midawson@redhat.com>
1 parent 83975b7 commit f7892c1

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
@@ -103,7 +103,8 @@ function makeRequireFunction(mod, redirects) {
103103
};
104104
} else {
105105
require = function require(path) {
106-
return mod[require_private_symbol](mod, path);
106+
// When no policy manifest, the original prototype.require is sustained
107+
return mod.require(path);
107108
};
108109
}
109110

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

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,10 +194,9 @@ function Module(id = '', parent) {
194194
if (policy?.manifest) {
195195
const moduleURL = pathToFileURL(id);
196196
redirects = policy.manifest.getDependencyMapper(moduleURL);
197+
// TODO(rafaelgss): remove the necessity of this branch
198+
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
197199
}
198-
setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
199-
// Loads a module at the given file path. Returns that module's
200-
// `exports` property.
201200
this[require_private_symbol] = internalRequire;
202201
}
203202

@@ -990,6 +989,23 @@ Module.prototype.load = function(filename) {
990989
ESMLoader.cjsCache.set(this, exports);
991990
};
992991

992+
// Loads a module at the given file path. Returns that module's
993+
// `exports` property.
994+
// When using the experimental policy mechanism this function is overridden
995+
Module.prototype.require = function(id) {
996+
validateString(id, 'id');
997+
if (id === '') {
998+
throw new ERR_INVALID_ARG_VALUE('id', id,
999+
'must be a non-empty string');
1000+
}
1001+
requireDepth++;
1002+
try {
1003+
return Module._load(id, this, /* isMain */ false);
1004+
} finally {
1005+
requireDepth--;
1006+
}
1007+
};
1008+
9931009
// Resolved path to process.argv[1] will be lazily placed here
9941010
// (needed for setting breakpoint when called with --inspect-brk)
9951011
let resolvedArgv;
@@ -1050,10 +1066,12 @@ function wrapSafe(filename, content, cjsModuleInstance) {
10501066
// Returns exception, if any.
10511067
Module.prototype._compile = function(content, filename) {
10521068
let moduleURL;
1053-
if (policy?.manifest) {
1069+
let redirects;
1070+
const manifest = policy?.manifest;
1071+
if (manifest) {
10541072
moduleURL = pathToFileURL(filename);
1055-
policy.manifest.getDependencyMapper(moduleURL);
1056-
policy.manifest.assertIntegrity(moduleURL, content);
1073+
redirects = manifest.getDependencyMapper(moduleURL);
1074+
manifest.assertIntegrity(moduleURL, content);
10571075
}
10581076

10591077
maybeCacheSourceMap(filename, content, this);
@@ -1083,17 +1101,18 @@ Module.prototype._compile = function(content, filename) {
10831101
}
10841102
}
10851103
const dirname = path.dirname(filename);
1104+
const require = makeRequireFunction(this, redirects);
10861105
let result;
10871106
const exports = this.exports;
10881107
const thisValue = exports;
10891108
const module = this;
10901109
if (requireDepth === 0) statCache = new SafeMap();
10911110
if (inspectorWrapper) {
10921111
result = inspectorWrapper(compiledWrapper, thisValue, exports,
1093-
module.require, module, filename, dirname);
1112+
require, module, filename, dirname);
10941113
} else {
10951114
result = ReflectApply(compiledWrapper, thisValue,
1096-
[exports, module.require, module, filename, dirname]);
1115+
[exports, require, module, filename, dirname]);
10971116
}
10981117
hasLoadedAnyUserCJSModule = true;
10991118
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 (internal/modules/cjs/loader.js:*)
99
at Module.load (internal/modules/cjs/loader.js:*)
1010
at Function.Module._load (internal/modules/cjs/loader.js:*)
11-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
11+
at Module.require (internal/modules/cjs/loader.js:*)
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 (internal/modules/cjs/loader.js:*)
2424
at Module.load (internal/modules/cjs/loader.js:*)
2525
at Function.Module._load (internal/modules/cjs/loader.js:*)
26-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
26+
at Module.require (internal/modules/cjs/loader.js:*)

‎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 (internal/modules/cjs/loader.js:*)
1313
at Module.load (internal/modules/cjs/loader.js:*)
1414
at Function.Module._load (internal/modules/cjs/loader.js:*)
15-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
15+
at Module.require (internal/modules/cjs/loader.js:*)
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 (internal/modules/cjs/loader.js:*)
2828
at Module.load (internal/modules/cjs/loader.js:*)
2929
at Function.Module._load (internal/modules/cjs/loader.js:*)
30-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
30+
at Module.require (internal/modules/cjs/loader.js:*)

‎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 (internal/modules/cjs/loader.js:*)
1313
at Module.load (internal/modules/cjs/loader.js:*)
1414
at Function.Module._load (internal/modules/cjs/loader.js:*)
15-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
15+
at Module.require (internal/modules/cjs/loader.js:*)

‎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 (internal/modules/cjs/loader.js:*
1010
at Module.load (internal/modules/cjs/loader.js:*
1111
at Function.Module._load (internal/modules/cjs/loader.js:*
12-
at Module.internalRequire (internal/modules/cjs/loader.js:*
12+
at Module.require (internal/modules/cjs/loader.js:*
1313
at require (internal/modules/cjs/helpers.js:*
1414
at Object.<anonymous> (*source_map_reference_error_tabs.js:*
1515
at Module._compile (internal/modules/cjs/loader.js:*

‎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 (internal/modules/cjs/loader.js:*)
1010
at Module.load (internal/modules/cjs/loader.js:*)
1111
at Function.Module._load (internal/modules/cjs/loader.js:*)
12-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
12+
at Module.require (internal/modules/cjs/loader.js:*)
1313
at require (internal/modules/cjs/helpers.js:*)
1414
at Object.<anonymous> (*source_map_throw_catch.js:6:3)
1515
at Module._compile (internal/modules/cjs/loader.js:*)

‎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 (internal/modules/cjs/loader.js:*)
1010
at Module.load (internal/modules/cjs/loader.js:*)
1111
at Function.Module._load (internal/modules/cjs/loader.js:*)
12-
at Module.internalRequire (internal/modules/cjs/loader.js:*)
12+
at Module.require (internal/modules/cjs/loader.js:*)
1313
at require (internal/modules/cjs/helpers.js:*)
1414
at Object.<anonymous> (*source_map_throw_first_tick.js:5:1)
1515
at Module._compile (internal/modules/cjs/loader.js:*)

‎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 (internal/modules/cjs/loader.js:*
1010
at Module.load (internal/modules/cjs/loader.js:*
1111
at Function.Module._load (internal/modules/cjs/loader.js:*
12-
at Module.internalRequire (internal/modules/cjs/loader.js:*
12+
at Module.require (internal/modules/cjs/loader.js:*
1313
at require (internal/modules/cjs/helpers.js:*
1414
at Object.<anonymous> (*source_map_throw_icu.js:*
1515
at Module._compile (internal/modules/cjs/loader.js:*

0 commit comments

Comments
 (0)