policy: handle mainModule.__proto__ bypass

RafaelGSS · RafaelGSS · commit d0a8264ec94d · 2023-06-19T12:10:41.000-03:00
PR-URL: nodejs-private/node-private#416 Fixes: https://hackerone.com/bugs?subject=nodejs&report_id=1877919 Reviewed-By: Rich Trott <rtrott@gmail.com> CVE-ID: CVE-2023-30581
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -231,6 +231,8 @@ function Module(id = '', parent) {
     redirects = manifest.getDependencyMapper(moduleURL);
     // TODO(rafaelgss): remove the necessity of this branch
     setOwnProperty(this, 'require', makeRequireFunction(this, redirects));
+    // eslint-disable-next-line no-proto
+    setOwnProperty(this.__proto__, 'require', makeRequireFunction(this, redirects));
   }
   this[require_private_symbol] = internalRequire;
 }
@@ -943,7 +945,7 @@ Module._load = function(request, parent, isMain) {
   const module = cachedModule || new Module(filename, parent);
 
   if (isMain) {
-    process.mainModule = module;
+    setOwnProperty(process, 'mainModule', module);
     setOwnProperty(module.require, 'main', process.mainModule);
     module.id = '.';
   }
diff --git a/test/fixtures/errors/force_colors.snapshot b/test/fixtures/errors/force_colors.snapshot
@@ -4,10 +4,10 @@ throw new Error('Should include grayed stack trace')
 
 Error: Should include grayed stack trace
     at Object.<anonymous> [90m(/[39mtest*force_colors.js:1:7[90m)[39m
-[90m    at Module._compile (node:internal*modules*cjs*loader:1255:14)[39m
-[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1309:10)[39m
-[90m    at Module.load (node:internal*modules*cjs*loader:1113:32)[39m
-[90m    at Module._load (node:internal*modules*cjs*loader:960:12)[39m
+[90m    at Module._compile (node:internal*modules*cjs*loader:1257:14)[39m
+[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1311:10)[39m
+[90m    at Module.load (node:internal*modules*cjs*loader:1115:32)[39m
+[90m    at Module._load (node:internal*modules*cjs*loader:962:12)[39m
 [90m    at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:83:12)[39m
 [90m    at node:internal*main*run_main_module:23:47[39m
 
diff --git a/test/fixtures/policy-manifest/main-module-proto-bypass.js b/test/fixtures/policy-manifest/main-module-proto-bypass.js
@@ -0,0 +1 @@
+process.mainModule.__proto__.require("os")
diff --git a/test/parallel/test-policy-manifest.js b/test/parallel/test-policy-manifest.js
@@ -66,3 +66,18 @@ const fixtures = require('../common/fixtures.js');
 
   assert.strictEqual(result.status, 0);
 }
+
+{
+  const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json');
+  const mainModuleBypass = fixtures.path('policy-manifest', 'main-module-proto-bypass.js');
+  const result = spawnSync(process.execPath, [
+    '--experimental-policy',
+    policyFilepath,
+    mainModuleBypass,
+  ]);
+
+  assert.notStrictEqual(result.status, 0);
+  const stderr = result.stderr.toString();
+  assert.match(stderr, /ERR_MANIFEST_DEPENDENCY_MISSING/);
+  assert.match(stderr, /does not list os as a dependency specifier for conditions: require, node, node-addons/);
+}

Original file line number	Diff line number	Diff line change
`@@ -231,6 +231,8 @@ function Module(id = '', parent) {`
`231`	`231`	`redirects = manifest.getDependencyMapper(moduleURL);`
`232`	`232`	`// TODO(rafaelgss): remove the necessity of this branch`
`233`	`233`	`setOwnProperty(this, 'require', makeRequireFunction(this, redirects));`
	`234`	`+ // eslint-disable-next-line no-proto`
	`235`	`+ setOwnProperty(this.__proto__, 'require', makeRequireFunction(this, redirects));`
`234`	`236`	`}`
`235`	`237`	`this[require_private_symbol] = internalRequire;`
`236`	`238`	`}`
`@@ -943,7 +945,7 @@ Module._load = function(request, parent, isMain) {`
`943`	`945`	`const module = cachedModule \|\| new Module(filename, parent);`
`944`	`946`
`945`	`947`	`if (isMain) {`
`946`		`- process.mainModule = module;`
	`948`	`+ setOwnProperty(process, 'mainModule', module);`
`947`	`949`	`setOwnProperty(module.require, 'main', process.mainModule);`
`948`	`950`	`module.id = '.';`
`949`	`951`	`}`
-Original file line number
+Diff line change
 Error: Should include grayed stack trace
     at Object.<anonymous> [90m(/[39mtest*force_colors.js:1:7[90m)[39m
 -[90m    at Module._compile (node:internal*modules*cjs*loader:1255:14)[39m
 -[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1309:10)[39m
 -[90m    at Module.load (node:internal*modules*cjs*loader:1113:32)[39m
 -[90m    at Module._load (node:internal*modules*cjs*loader:960:12)[39m
 +[90m    at Module._compile (node:internal*modules*cjs*loader:1257:14)[39m
 +[90m    at Module._extensions..js (node:internal*modules*cjs*loader:1311:10)[39m
 +[90m    at Module.load (node:internal*modules*cjs*loader:1115:32)[39m
 +[90m    at Module._load (node:internal*modules*cjs*loader:962:12)[39m
 [90m    at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:83:12)[39m
 [90m    at node:internal*main*run_main_module:23:47[39m
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+process.mainModule.__proto__.require("os")`