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

Add --give-projects-full-access-to-my-computer-including-ability-to-install-malware #1112

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 32 additions & 12 deletions src-main/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ require('./context-menu');
require('./menu-bar');
require('./crash-messages');

app.enableSandbox();

// Allows certain versions of Scratch Link to work without an internet connection
// https://github.com/LLK/scratch-desktop/blob/4b462212a8e406b15bcf549f8523645602b46064/src/main/index.js#L45
app.commandLine.appendSwitch('host-resolver-rules', 'MAP device-manager.scratch.mit.edu 127.0.0.1');
Expand Down Expand Up @@ -130,7 +128,7 @@ app.on('web-contents-created', (event, webContents) => {
});

app.on('window-all-closed', () => {
if (!isMigrating) {
if (!isInitializing) {
app.quit();
}
});
Expand All @@ -149,15 +147,22 @@ app.on('open-file', (event, path) => {
// This event can be called before ready.
if (app.isReady() && !isMigrating) {
// The path we get should already be absolute
EditorWindow.openFiles([path], '');
EditorWindow.openPaths([path], false, false, null);
} else {
filesQueuedToOpen.push(path);
}
});

/**
* @typedef ParsedCommandLine
* @property {string[]} files
* @property {boolean} fullscreen
* @property {boolean} nodeIntegration
*/

/**
* @param {string[]} argv
* @returns {{files: string[]; fullscreen: boolean;}}
* @returns {ParsedCommandLine}
*/
const parseCommandLine = (argv) => {
// argv could be any of:
Expand All @@ -176,27 +181,35 @@ const parseCommandLine = (argv) => {
.slice(process.defaultApp ? 2 : 1);

const fullscreen = argv.includes('--fullscreen');
const nodeIntegration = argv.includes('--give-projects-full-access-to-my-computer-including-ability-to-install-malware');

return {
files,
fullscreen
fullscreen,
nodeIntegration
};
};

let isMigrating = true;
let isInitializing = true;
let migratePromise = null;

app.on('second-instance', (event, argv, workingDirectory) => {
migratePromise.then(() => {
const commandLineOptions = parseCommandLine(argv);
EditorWindow.openFiles(commandLineOptions.files, commandLineOptions.fullscreen, workingDirectory);
EditorWindow.openPaths(
commandLineOptions.files,
commandLineOptions.fullscreen,
commandLineOptions.nodeIntegration,
workingDirectory
);
});
});

app.whenReady().then(() => {
AbstractWindow.settingsChanged();

migratePromise = migrate().then((shouldContinue) => {
migratePromise = migrate().then(async (shouldContinue) => {
if (!shouldContinue) {
// If we use exit() instead of quit() then openExternal() calls made before the app quits
// won't work on Windows.
Expand All @@ -207,10 +220,17 @@ app.whenReady().then(() => {
isMigrating = false;

const commandLineOptions = parseCommandLine(process.argv);
EditorWindow.openFiles([
...filesQueuedToOpen,
...commandLineOptions.files
], commandLineOptions.fullscreen, process.cwd());
await EditorWindow.openPaths(
[
...filesQueuedToOpen,
...commandLineOptions.files
],
commandLineOptions.fullscreen,
commandLineOptions.nodeIntegration,
process.cwd()
);

isInitializing = false;

if (AbstractWindow.getAllWindows().length === 0) {
// No windows were successfully opened. Let's just quit.
Expand Down
34 changes: 25 additions & 9 deletions src-main/l10n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -424,36 +424,48 @@
"developer_comment": "Appears in file saving dialog when saving some types of packaged projects"
},
"security-prompt.title": {
"string": "Extension Security",
"developer_comment": "Title of extension security prompt window"
"string": "Security",
"developer_comment": "Title of desktop app's security prompt window"
},
"security-prompt.allow": {
"string": "Allow",
"developer_comment": "Title of extension security prompt window. This button allows the permission."
"developer_comment": "Title of desktop app's security prompt window. This button grants the request."
},
"security-prompt.deny": {
"string": "Deny",
"developer_comment": "Title of extension security prompt window. This button denies the permission."
"developer_comment": "Title of desktop app's security prompt window. This button denies the request."
},
"security-prompt.node-integration1": {
"string": "{APP_NAME} was started in a special mode that will give any opened project unrestricted access to your computer. This would allow stealing passwords and installing malware.",
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.node-integration2": {
"string": "The {APP_NAME} developers are not responsible for any damages.",
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.node-integration3": {
"string": "If you were not expecting to see this screen, please click \"Deny\" to safely exit.",
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.read-clipboard1": {
"string": "The project wants to read data from your clipboard.",
"developer_comment": "Part of the extension security prompt window"
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.read-clipboard2": {
"string": "If your clipboard contains things like passwords, the project may be able to share those with other users or servers.",
"developer_comment": "Part of the extension security prompt window"
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.read-clipboard3": {
"string": "Clipboard access may not work on some systems. If allowed, further clipboard reads will be automatically allowed.",
"developer_comment": "Part of the extension security prompt window"
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.notifications1": {
"string": "The project wants to display notifications.",
"developer_comment": "Part of the extension security prompt window"
"developer_comment": "Part of the desktop app's security prompt window"
},
"security-prompt.notifications2": {
"string": "If allowed, you may be prompted to enable notifications by your operating system, and further notifications will be automatically allowed.",
"developer_comment": "Part of the extension security prompt window"
"developer_comment": "Part of the desktop app's security prompt window"
},
"unsafe-path.title": {
"string": "Invalid File Location",
Expand Down Expand Up @@ -486,5 +498,9 @@
"extension-documentation.title": {
"string": "{APP_NAME} Extension Documentation",
"developer_comment": "Title of in-app window for viewing extension documentation."
},
"node-integration.prefix": {
"string": "Unrestricted System Access",
"developer_comment": "Appears in titles of desktop app windows that have been given full access to the user's sytem"
}
}
15 changes: 11 additions & 4 deletions src-main/windows/abstract.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const windowsByClass = new Map();
* @typedef AbstractWindowOptions
* @property {Electron.BrowserWindow} [existingWindow]
* @property {Electron.BrowserWindow} [parentWindow]
* @property {boolean} [nodeIntegration]
*/

class AbstractWindow {
Expand All @@ -18,7 +19,7 @@ class AbstractWindow {
this.parentWindow = options.parentWindow || null;

/** @type {Electron.BrowserWindow} */
this.window = options.existingWindow || new BrowserWindow(this.getWindowOptions());
this.window = options.existingWindow || new BrowserWindow(this.getWindowOptions(options));
this.window.webContents.on('before-input-event', this.handleInput.bind(this));
this.applySettings();

Expand Down Expand Up @@ -163,7 +164,10 @@ class AbstractWindow {
return '#ffffff';
}

getWindowOptions () {
/**
* @param {AbstractWindowOptions} constructorOptions
*/
getWindowOptions (constructorOptions) {
/** @type {Electron.BrowserWindowConstructorOptions} */
const options = {};

Expand All @@ -174,8 +178,11 @@ class AbstractWindow {
// Child classes are expected to show the window on their own
options.show = false;

// These should all be redundant already, but defense-in-depth.
options.webPreferences = {
options.webPreferences = constructorOptions.nodeIntegration ? {
nodeIntegration: true,
contextIsolation: false,
sandbox: false,
} : {
nodeIntegration: false,
contextIsolation: true,
sandbox: true,
Expand Down
51 changes: 33 additions & 18 deletions src-main/windows/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ const privilegedFetch = require('../fetch');
const RichPresence = require('../rich-presence.js');
const FileAccessWindow = require('./file-access-window.js');
const ExtensionDocumentationWindow = require('./extension-documentation.js');
const SecurityPromptWindow = require('./security-prompt.js');

const TYPE_FILE = 'file';
const TYPE_URL = 'url';
Expand Down Expand Up @@ -215,9 +216,12 @@ class EditorWindow extends ProjectRunningWindow {
/**
* @param {OpenedFile|null} initialFile
* @param {boolean} isInitiallyFullscreen
* @param {boolean} nodeIntegration
*/
constructor (initialFile, isInitiallyFullscreen) {
super();
constructor (initialFile, isInitiallyFullscreen, nodeIntegration) {
super({
nodeIntegration
});

/**
* Ideally we would revoke access after loading a new project, but our file handle handling in
Expand Down Expand Up @@ -267,19 +271,20 @@ class EditorWindow extends ProjectRunningWindow {
}
});

const titlePrefix = nodeIntegration ? `[${translate('node-integration.prefix')}] ` : '';
this.window.on('page-title-updated', (event, title, explicitSet) => {
event.preventDefault();
if (explicitSet && title) {
this.window.setTitle(`${title} - ${APP_NAME}`);
this.window.setTitle(`${titlePrefix}${title} - ${APP_NAME}`);
this.projectTitle = title;
} else {
this.window.setTitle(APP_NAME);
this.window.setTitle(`${titlePrefix}${APP_NAME}`);
this.projectTitle = '';
}

this.updateRichPresence();
});
this.window.setTitle(APP_NAME);
this.window.setTitle(`${titlePrefix}${APP_NAME}`);

this.window.on('focus', () => {
this.updateRichPresence();
Expand Down Expand Up @@ -504,7 +509,7 @@ class EditorWindow extends ProjectRunningWindow {
});

this.ipc.handle('open-new-window', () => {
EditorWindow.newWindow();
EditorWindow.newWindow(null, false, false);
});

this.ipc.handle('open-addon-settings', (event, search) => {
Expand Down Expand Up @@ -602,7 +607,7 @@ class EditorWindow extends ProjectRunningWindow {
const projectUrl = params.get('project_url');
const parsedFile = parseOpenedFile(projectUrl, null);
if (parsedFile.type === TYPE_SAMPLE) {
new EditorWindow(parsedFile, null);
EditorWindow.newWindow(parsedFile, false, false);
return {
action: 'deny'
};
Expand Down Expand Up @@ -632,26 +637,36 @@ class EditorWindow extends ProjectRunningWindow {
}

/**
* @param {string[]} files
* @param {string[]} paths
* @param {boolean} fullscreen
* @param {boolean} nodeIntegration
* @param {string|null} workingDirectory
* @returns {Promise<void>}
*/
static openFiles (files, fullscreen, workingDirectory) {
if (files.length === 0) {
EditorWindow.newWindow(fullscreen);
} else {
for (const file of files) {
new EditorWindow(parseOpenedFile(file, workingDirectory), fullscreen);
}
static openPaths (paths, fullscreen, nodeIntegration, workingDirectory) {
if (paths.length === 0) {
return EditorWindow.newWindow(null, fullscreen, nodeIntegration);
}
return Promise.all(paths.map(path => (
EditorWindow.newWindow(parseOpenedFile(path, workingDirectory), fullscreen, nodeIntegration)
)));
}

/**
* Open a new window with the default project.
* Try to open a new window.
* @param {OpenedFile|null} file
* @param {boolean} fullscreen
* @param {boolean} nodeIntegration
* @returns {Promise<void>}
*/
static newWindow (fullscreen) {
new EditorWindow(null, fullscreen);
static async newWindow (file, fullscreen, nodeIntegration) {
if (nodeIntegration) {
const allowed = await SecurityPromptWindow.requestNodeIntegration();
if (!allowed) {
return;
}
}
new EditorWindow(file, fullscreen, nodeIntegration);
}
}

Expand Down
11 changes: 10 additions & 1 deletion src-main/windows/security-prompt.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,16 @@ class SecurityState {

class SecurityPromptWindow extends AbstractWindow {
/**
* @param {Electron.BrowserWindow} projectWindow
* @param {Electron.BrowserWindow|null} projectWindow
* @param {string} type
*/
constructor (projectWindow, type) {
super({
parentWindow: projectWindow
});

this.type = type;

/** @type {Promise<boolean>} */
this.promptPromise = new Promise((resolve) => {
this.promptResolve = resolve;
Expand Down Expand Up @@ -130,6 +132,13 @@ class SecurityPromptWindow extends AbstractWindow {
return this.promptPromise;
}

static async requestNodeIntegration () {
const securityWindows = AbstractWindow.getWindowsByClass(SecurityPromptWindow);
const existingWindow = securityWindows.find(i => i.type === 'node-integration');
const window = existingWindow || new SecurityPromptWindow(null, 'node-integration');
return window.done();
}

static async requestReadClipboard (window) {
const state = SecurityState.forWindow(window);
if (!state.allowedReadClipboard) {
Expand Down
13 changes: 11 additions & 2 deletions src-preload/editor.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
const {contextBridge, ipcRenderer} = require('electron');

contextBridge.exposeInMainWorld('EditorPreload', {
const exposeInMainWorld = (name, api) => {
// TODO: find a better way to do this
try {
contextBridge.exposeInMainWorld(name, api);
} catch (e) {
global[name] = api;
}
};

exposeInMainWorld('EditorPreload', {
isInitiallyFullscreen: () => ipcRenderer.sendSync('is-initially-fullscreen'),
getInitialFile: () => ipcRenderer.invoke('get-initial-file'),
getFile: (id) => ipcRenderer.invoke('get-file', id),
Expand Down Expand Up @@ -66,7 +75,7 @@ ipcRenderer.on('enumerate-media-devices', (e) => {
});
});

contextBridge.exposeInMainWorld('PromptsPreload', {
exposeInMainWorld('PromptsPreload', {
alert: (message) => ipcRenderer.sendSync('alert', message),
confirm: (message) => ipcRenderer.sendSync('confirm', message),
});
Expand Down
4 changes: 4 additions & 0 deletions src-renderer/security-prompt/security-prompt.html
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,10 @@

<body>
<main>
<p data-type="node-integration" data-l10n="security-prompt.node-integration1"></p>
<p data-type="node-integration" data-l10n="security-prompt.node-integration2"></p>
<p data-type="node-integration" data-l10n="security-prompt.node-integration3"></p>

<p data-type="read-clipboard" data-l10n="security-prompt.read-clipboard1"></p>
<p data-type="read-clipboard" data-l10n="security-prompt.read-clipboard2"></p>
<p data-type="read-clipboard" data-l10n="security-prompt.read-clipboard3"></p>
Expand Down
Loading