policy: handle Module.constructor and main.extensions bypass

RafaelGSS · RafaelGSS · commit d8ccfe9ad4dc · 2023-08-08T10:00:22.000-03:00
Signed-off-by: RafaelGSS <rafael.nunu@hotmail.com> PR-URL: nodejs-private/node-private#445 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=1960870 Refs: https://hackerone.com/bugs?subject=nodejs&report_id=2043807 CVE-ID: CVE-2023-32002,CVE-2023-32006
diff --git a/lib/internal/modules/cjs/helpers.js b/lib/internal/modules/cjs/helpers.js
@@ -14,6 +14,7 @@ const {
   StringPrototypeStartsWith,
 } = primordials;
 const {
+  ERR_INVALID_ARG_TYPE,
   ERR_MANIFEST_DEPENDENCY_MISSING,
   ERR_UNKNOWN_BUILTIN_MODULE
 } = require('internal/errors').codes;
@@ -56,12 +57,22 @@ function loadBuiltinModule(filename, request) {
   }
 }
 
+let $Module = null;
+function lazyModule() {
+  $Module = $Module || require('internal/modules/cjs/loader').Module;
+  return $Module;
+}
+
 // Invoke with makeRequireFunction(module) where |module| is the Module object
 // to use as the context for the require() function.
 // Use redirects to set up a mapping from a policy and restrict dependencies
 const urlToFileCache = new SafeMap();
 function makeRequireFunction(mod, redirects) {
-  const Module = mod.constructor;
+  // lazy due to cycle
+  const Module = lazyModule();
+  if (mod instanceof Module !== true) {
+    throw new ERR_INVALID_ARG_TYPE('mod', 'Module', mod);
+  }
 
   let require;
   if (redirects) {
diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js
@@ -155,8 +155,8 @@ const isWindows = process.platform === 'win32';
 const relativeResolveCache = ObjectCreate(null);
 
 let requireDepth = 0;
-let statCache = null;
 let isPreloading = false;
+let statCache = null;
 
 function internalRequire(module, id) {
   validateString(id, 'id');
@@ -1389,5 +1389,14 @@ Module.isBuiltin = function isBuiltin(moduleName) {
   return allBuiltins.has(moduleName);
 };
 
+ObjectDefineProperty(Module.prototype, 'constructor', {
+  __proto__: null,
+  get: function() {
+    return policy ? undefined : Module;
+  },
+  configurable: false,
+  enumerable: false,
+});
+
 // Backwards compatibility
 Module.Module = Module;
diff --git a/test/fixtures/policy-manifest/createRequire-bypass.js b/test/fixtures/policy-manifest/createRequire-bypass.js
@@ -0,0 +1,2 @@
+const os = module.constructor.createRequire('file:///os-access-module.js')('os')
+os.cpus()
diff --git a/test/fixtures/policy-manifest/invalid-module.js b/test/fixtures/policy-manifest/invalid-module.js
diff --git a/test/fixtures/policy-manifest/main-constructor-bypass.js b/test/fixtures/policy-manifest/main-constructor-bypass.js
@@ -0,0 +1,2 @@
+const m = new require.main.constructor();
+m.require('./invalid-module')
diff --git a/test/fixtures/policy-manifest/main-constructor-extensions-bypass.js b/test/fixtures/policy-manifest/main-constructor-extensions-bypass.js
@@ -0,0 +1,2 @@
+const m = new require.main.constructor();
+require.extensions['.js'](m, './invalid-module')
diff --git a/test/fixtures/policy-manifest/manifest-impersonate.json b/test/fixtures/policy-manifest/manifest-impersonate.json
@@ -0,0 +1,13 @@
+{
+  "resources": {
+    "./createRequire-bypass.js": {
+      "integrity": true
+    },
+    "/os-access-module.js": {
+      "integrity": true,
+      "dependencies": {
+        "os": true
+      }
+    }
+  }
+}
diff --git a/test/fixtures/policy-manifest/module-constructor-bypass.js b/test/fixtures/policy-manifest/module-constructor-bypass.js
@@ -0,0 +1 @@
+module.constructor._load('node:child_process');
diff --git a/test/parallel/test-policy-manifest.js b/test/parallel/test-policy-manifest.js
@@ -81,3 +81,58 @@ const fixtures = require('../common/fixtures.js');
   assert.match(stderr, /ERR_MANIFEST_DEPENDENCY_MISSING/);
   assert.match(stderr, /does not list os as a dependency specifier for conditions: require, node, node-addons/);
 }
+
+{
+  const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json');
+  const mainModuleBypass = fixtures.path('policy-manifest', 'module-constructor-bypass.js');
+  const result = spawnSync(process.execPath, [
+    '--experimental-policy',
+    policyFilepath,
+    mainModuleBypass,
+  ]);
+  assert.notStrictEqual(result.status, 0);
+  const stderr = result.stderr.toString();
+  assert.match(stderr, /TypeError/);
+}
+
+{
+  const policyFilepath = fixtures.path('policy-manifest', 'manifest-impersonate.json');
+  const createRequireBypass = fixtures.path('policy-manifest', 'createRequire-bypass.js');
+  const result = spawnSync(process.execPath, [
+    '--experimental-policy',
+    policyFilepath,
+    createRequireBypass,
+  ]);
+
+  assert.notStrictEqual(result.status, 0);
+  const stderr = result.stderr.toString();
+  assert.match(stderr, /TypeError/);
+}
+
+{
+  const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json');
+  const mainModuleBypass = fixtures.path('policy-manifest', 'main-constructor-bypass.js');
+  const result = spawnSync(process.execPath, [
+    '--experimental-policy',
+    policyFilepath,
+    mainModuleBypass,
+  ]);
+
+  assert.notStrictEqual(result.status, 0);
+  const stderr = result.stderr.toString();
+  assert.match(stderr, /TypeError/);
+}
+
+{
+  const policyFilepath = fixtures.path('policy-manifest', 'onerror-exit.json');
+  const mainModuleBypass = fixtures.path('policy-manifest', 'main-constructor-extensions-bypass.js');
+  const result = spawnSync(process.execPath, [
+    '--experimental-policy',
+    policyFilepath,
+    mainModuleBypass,
+  ]);
+
+  assert.notStrictEqual(result.status, 0);
+  const stderr = result.stderr.toString();
+  assert.match(stderr, /TypeError/);
+}

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const os = module.constructor.createRequire('file:///os-access-module.js')('os')`
	`2`	`+os.cpus()`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const m = new require.main.constructor();`
	`2`	`+m.require('./invalid-module')`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+const m = new require.main.constructor();`
	`2`	`+require.extensions['.js'](m, './invalid-module')`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+module.constructor._load('node:child_process');`