Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: apply module search paths restriction on worker and child process #41137

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions filenames.auto.gni
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ auto_filenames = {
"lib/common/deprecate.ts",
"lib/common/init.ts",
"lib/common/ipc-messages.ts",
"lib/common/reset-search-paths.ts",
"lib/common/web-view-methods.ts",
"lib/common/webpack-globals-provider.ts",
"package.json",
Expand All @@ -271,7 +270,6 @@ auto_filenames = {
"lib/common/define-properties.ts",
"lib/common/init.ts",
"lib/common/ipc-messages.ts",
"lib/common/reset-search-paths.ts",
"lib/common/web-view-methods.ts",
"lib/common/webpack-provider.ts",
"lib/renderer/api/clipboard.ts",
Expand Down Expand Up @@ -309,7 +307,6 @@ auto_filenames = {
"lib/common/define-properties.ts",
"lib/common/init.ts",
"lib/common/ipc-messages.ts",
"lib/common/reset-search-paths.ts",
"lib/common/webpack-provider.ts",
"lib/renderer/api/clipboard.ts",
"lib/renderer/api/context-bridge.ts",
Expand Down Expand Up @@ -344,7 +341,6 @@ auto_filenames = {
"lib/common/api/net-client-request.ts",
"lib/common/define-properties.ts",
"lib/common/init.ts",
"lib/common/reset-search-paths.ts",
"lib/common/webpack-globals-provider.ts",
"lib/utility/api/exports/electron.ts",
"lib/utility/api/module-list.ts",
Expand Down
3 changes: 0 additions & 3 deletions lib/browser/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ const Module = require('module') as NodeJS.ModuleInternal;
// we need to restore it here.
process.argv.splice(1, 1);

// Clear search paths.
require('../common/reset-search-paths');

// Import common settings.
require('@electron/internal/common/init');

Expand Down
40 changes: 40 additions & 0 deletions lib/common/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,43 @@ if (process.platform === 'win32') {
}
});
}

const Module = require('module') as NodeJS.ModuleInternal;

// Make a fake Electron module that we will insert into the module cache
const makeElectronModule = (name: string) => {
const electronModule = new Module('electron', null);
electronModule.id = 'electron';
electronModule.loaded = true;
electronModule.filename = name;
Object.defineProperty(electronModule, 'exports', {
get: () => require('electron')
});
Module._cache[name] = electronModule;
};

makeElectronModule('electron');
makeElectronModule('electron/common');
if (process.type === 'browser') {
makeElectronModule('electron/main');
}
if (process.type === 'renderer') {
makeElectronModule('electron/renderer');
}

const originalResolveFilename = Module._resolveFilename;

// 'electron/main', 'electron/renderer' and 'electron/common' are module aliases
// of the 'electron' module for TypeScript purposes, i.e., the types for
// 'electron/main' consist of only main process modules, etc. It is intentional
// that these can be `require()`-ed from both the main process as well as the
// renderer process regardless of the names, they're superficial for TypeScript
// only.
const electronModuleNames = new Set(['electron', 'electron/main', 'electron/renderer', 'electron/common']);
Module._resolveFilename = function (request, parent, isMain, options) {
if (electronModuleNames.has(request)) {
return 'electron';
} else {
return originalResolveFilename(request, parent, isMain, options);
}
};
69 changes: 0 additions & 69 deletions lib/common/reset-search-paths.ts

This file was deleted.

18 changes: 18 additions & 0 deletions lib/node/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,21 @@ cp.fork = (modulePath: string, args: any, options: any) => {
}
return originalFork(modulePath, args, options);
};

// Prevent Node from adding paths outside this app to search paths.
const path = require('path');
const Module = require('module') as NodeJS.ModuleInternal;
const resourcesPathWithTrailingSlash = process.resourcesPath + path.sep;
const originalNodeModulePaths = Module._nodeModulePaths;
Module._nodeModulePaths = function (from: string) {
const paths: string[] = originalNodeModulePaths(from);
const fromPath = path.resolve(from) + path.sep;
// If "from" is outside the app then we do nothing.
if (fromPath.startsWith(resourcesPathWithTrailingSlash)) {
return paths.filter(function (candidate) {
return candidate.startsWith(resourcesPathWithTrailingSlash);
});
} else {
return paths;
}
};
13 changes: 10 additions & 3 deletions lib/renderer/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,16 @@ import type * as ipcRendererUtilsModule from '@electron/internal/renderer/ipc-re

const Module = require('module') as NodeJS.ModuleInternal;

// We do not want to allow use of the VM module in the renderer process as
// it conflicts with Blink's V8::Context internal logic.
const originalModuleLoad = Module._load;
Module._load = function (request: string) {
if (request === 'vm') {
console.warn('The vm module of Node.js is deprecated in the renderer process and will be removed.');
}
return originalModuleLoad.apply(this, arguments as any);
};

// Make sure globals like "process" and "global" are always available in preload
// scripts even after they are deleted in "loaded" script.
//
Expand All @@ -33,9 +43,6 @@ Module.wrapper = [
// init.js, we need to restore it here.
process.argv.splice(1, 1);

// Clear search paths.
require('../common/reset-search-paths');

// Import common settings.
require('@electron/internal/common/init');

Expand Down
3 changes: 0 additions & 3 deletions lib/utility/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ const entryScript: string = v8Util.getHiddenValue(process, '_serviceStartupScrip
// we need to restore it here.
process.argv.splice(1, 1, entryScript);

// Clear search paths.
require('../common/reset-search-paths');

// Import common settings.
require('@electron/internal/common/init');

Expand Down
3 changes: 0 additions & 3 deletions lib/worker/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,6 @@ const Module = require('module') as NodeJS.ModuleInternal;
// init.js, we need to restore it here.
process.argv.splice(1, 1);

// Clear search paths.
require('../common/reset-search-paths');

// Import common settings.
require('@electron/internal/common/init');

Expand Down
2 changes: 2 additions & 0 deletions spec/fixtures/module/module-paths.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const Module = require('node:module');
process.send(Module._nodeModulePaths(process.resourcesPath + '/test.js'));
6 changes: 6 additions & 0 deletions spec/node-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ describe('node feature', () => {
const [msg] = await message;
expect(msg).to.equal('message');
});

it('Has its module searth paths restricted', async () => {
const child = childProcess.fork(path.join(fixtures, 'module', 'module-paths.js'));
const [msg] = await once(child, 'message');
expect(msg.length).to.equal(2);
});
});
});

Expand Down