Merge pull request from GHSA-5gpx-9976-ggpm

electron/html/about.html

@@ -11,10 +11,14 @@ <h1 id="name"></h1>
       <p class="copy"><span data-string="aboutVersion"></span> <span id="version"></span></p>
       <p class="webappVersion copy"><span data-string="aboutWebappVersion"></span> <span id="webappVersion"></span></p>
       <p>
-        <a href="https://support.wire.com/hc/articles/115001919905"><span data-string="aboutUpdate"></span></a>
+        <a href="https://support.wire.com/hc/articles/115001919905" target="_blank">
+          <span data-string="aboutUpdate"></span>
+        </a>
       </p>
       <p>
-        <a href="https://medium.com/wire-news/webapp-updates/home"><span data-string="aboutReleases"></span></a>
+        <a href="https://medium.com/wire-news/webapp-updates/home" target="_blank">
+          <span data-string="aboutReleases"></span>
+        </a>
       </p>
       <p><span id="copyright"></span></p>
     </div>

electron/src/lib/showDialog.ts

@@ -18,6 +18,7 @@
  */
 
 import {dialog, MessageBoxSyncOptions} from 'electron';
+import * as locale from '../locale/locale';
 
 export const showDialog = (message: string, title: string, type?: string): void => {
   const options: MessageBoxSyncOptions = {
@@ -31,3 +32,13 @@ export const showDialog = (message: string, title: string, type?: string): void
 export const showErrorDialog = (message: string): void => {
   showDialog(message, 'Error', 'error');
 };
+
+export const showWarningDialog = (message: string): void => {
+  const options: MessageBoxSyncOptions = {
+    buttons: ['OK'],
+    message,
+    title: 'Warning',
+    type: 'warning',
+  };
+  dialog.showMessageBoxSync(options);
+};

electron/src/main.ts

@@ -28,7 +28,6 @@ import {
   ipcMain,
   Menu,
   OnHeadersReceivedListenerDetails,
-  shell,
   WebContents,
 } from 'electron';
 import * as fs from 'fs-extra';
@@ -264,7 +263,7 @@ const showMainWindow = async (mainWindowState: windowStateKeeper.State): Promise
       return;
     }
 
-    await shell.openExternal(url);
+    return WindowUtil.openExternal(url);
   });
 
   main.on('focus', () => {
@@ -507,7 +506,7 @@ class ElectronWrapperInit {
       }
 
       this.logger.log('Opening an external window from a webview.');
-      return shell.openExternal(url);
+      return WindowUtil.openExternal(url);
     };
 
     const willNavigateInWebview = (event: ElectronEvent, url: string, baseUrl: string): void => {

electron/src/menu/system.ts

@@ -18,7 +18,7 @@
  */
 
 import autoLaunch = require('auto-launch');
-import {dialog, globalShortcut, ipcMain, Menu, MenuItemConstructorOptions, shell} from 'electron';
+import {dialog, globalShortcut, ipcMain, Menu, MenuItemConstructorOptions} from 'electron';
 import * as path from 'path';
 
 import {EVENT_TYPE} from '../lib/eventType';
@@ -33,6 +33,7 @@ import {SettingsType} from '../settings/SettingsType';
 import {WindowManager} from '../window/WindowManager';
 import {downloadLogs} from '../lib/download';
 import {zipFiles, createFile} from '../lib/zip';
+import * as WindowUtil from '../window/WindowUtil';
 
 const launchCmd = process.env.APPIMAGE || process.execPath;
 
@@ -251,23 +252,23 @@ const helpTemplate: MenuItemConstructorOptions = {
   role: 'help',
   submenu: [
     {
-      click: () => shell.openExternal(config.legalUrl),
+      click: () => WindowUtil.openExternal(config.legalUrl, true),
       label: locale.getText('menuLegal'),
     },
     {
-      click: () => shell.openExternal(config.privacyUrl),
+      click: () => WindowUtil.openExternal(config.privacyUrl, true),
       label: locale.getText('menuPrivacy'),
     },
     {
-      click: () => shell.openExternal(config.licensesUrl),
+      click: () => WindowUtil.openExternal(config.licensesUrl, true),
       label: locale.getText('menuLicense'),
     },
     {
-      click: () => shell.openExternal(config.supportUrl),
+      click: () => WindowUtil.openExternal(config.supportUrl, true),
       label: locale.getText('menuSupport'),
     },
     {
-      click: () => shell.openExternal(EnvironmentUtil.web.getWebsiteUrl()),
+      click: () => WindowUtil.openExternal(EnvironmentUtil.web.getWebsiteUrl(), true),
       label: locale.getText('menuAppURL'),
     },
     downloadLogsTemplate,

electron/src/window/AboutWindow.ts

@@ -17,7 +17,7 @@
  *
  */
 
-import {app, BrowserWindow, ipcMain, session, shell} from 'electron';
+import {app, BrowserWindow, ipcMain, session} from 'electron';
 import fileUrl = require('file-url');
 import * as path from 'path';
 
@@ -26,6 +26,7 @@ import * as locale from '../locale/locale';
 import * as EnvironmentUtil from '../runtime/EnvironmentUtil';
 import {config} from '../settings/config';
 import {getLogger} from '../logging/getLogger';
+import * as WindowUtil from '../window/WindowUtil';
 
 const logger = getLogger(path.basename(__filename));
 
@@ -89,14 +90,19 @@ const showWindow = async () => {
       if (ABOUT_WINDOW_ALLOWLIST.includes(url)) {
         return callback({cancel: false});
       }
+    });
+
+    // Handle the new window event in the About Window
+    aboutWindow.webContents.on('new-window', (event, url) => {
+      event.preventDefault();
 
-      // Open HTTPS links in browser instead
-      if (url.startsWith('https://')) {
-        await shell.openExternal(url);
-      } else {
-        logger.info(`Attempt to open URL "${url}" in window prevented.`);
-        callback({redirectURL: ABOUT_HTML});
+      // Ensure the link does not come from a webview
+      if (typeof (event as any).sender.viewInstanceId !== 'undefined') {
+        logger.log('New window was created from a webview, aborting.');
+        return;
       }
+
+      return WindowUtil.openExternal(url, true);
     });
 
     // Locales

electron/src/window/WindowUtil.test.ts

@@ -0,0 +1,73 @@
+/*
+ * Wire
+ * Copyright (C) 2020 Wire Swiss GmbH
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see http://www.gnu.org/licenses/.
+ *
+ */
+
+import {dialog, shell} from 'electron';
+import * as assert from 'assert';
+import * as sinon from 'sinon';
+
+import * as WindowUtil from './WindowUtil';
+
+describe('WindowUtil', () => {
+  describe('openExternal', () => {
+    let dialogStub: sinon.SinonStub;
+    let shellStub: sinon.SinonStub;
+
+    beforeEach(() => {
+      shellStub = sinon.stub(shell, 'openExternal').returns(Promise.resolve());
+      dialogStub = sinon.stub(dialog, 'showMessageBoxSync').returns(0);
+    });
+
+    afterEach(() => {
+      dialogStub.restore();
+      shellStub.restore();
+    });
+
+    it('opens secure URLs externally', async () => {
+      const url = 'https://github.com/wireapp/wire-desktop';
+      await WindowUtil.openExternal(url);
+
+      assert.ok(shellStub.firstCall.calledWith(url));
+      assert.ok(dialogStub.notCalled);
+    });
+
+    it('opens insecure URLs externally if allowed', async () => {
+      const url = 'http://github.com/wireapp/wire-desktop';
+      await WindowUtil.openExternal(url, false);
+
+      assert.ok(shellStub.firstCall.calledWith(url));
+      assert.ok(dialogStub.notCalled);
+    });
+
+    it(`doesn't open insecure URLs externally if not allowed`, async () => {
+      const url = 'http://example.org';
+      await WindowUtil.openExternal(url, true);
+
+      assert.ok(shellStub.notCalled);
+      assert.ok(dialogStub.called);
+    });
+
+    it(`doesn't open non-website URLs externally`, async () => {
+      const url = 'smb://attacker.tld/public/pwn.desktop';
+      await WindowUtil.openExternal(url);
+
+      assert.ok(shellStub.notCalled);
+      assert.ok(dialogStub.called);
+    });
+  });
+});

electron/src/window/WindowUtil.ts

@@ -17,7 +17,14 @@
  *
  */
 
-import {BrowserWindow, screen} from 'electron';
+import {BrowserWindow, screen, shell} from 'electron';
+import * as path from 'path';
+import * as URL from 'url';
+
+import {getLogger} from '../logging/getLogger';
+import {showWarningDialog} from '../lib/showDialog';
+
+const logger = getLogger(path.basename(__filename));
 
 interface Rectangle {
   height: number;
@@ -46,3 +53,24 @@ export const isInView = (win: BrowserWindow): boolean => {
 
   return upperLeftVisible || lowerRightVisible;
 };
+
+export const openExternal = async (url: string, httpsOnly: boolean = false): Promise<void> => {
+  try {
+    const urlProtocol = URL.parse(url).protocol || '';
+    const allowedProtocols = ['https:'];
+
+    if (!httpsOnly) {
+      allowedProtocols.push('ftp:', 'http:', 'mailto:');
+    }
+
+    if (!allowedProtocols.includes(urlProtocol)) {
+      logger.warn(`Prevented opening external URL "${url}".`);
+      showWarningDialog(`Prevented opening external URL "${url}".`);
+      return;
+    }
+
+    await shell.openExternal(url);
+  } catch (error) {
+    logger.error(error);
+  }
+};

yarn.lock

@@ -5345,16 +5345,6 @@ fs-extra-p@^8.0.2:
     bluebird-lst "^1.0.9"
     fs-extra "^8.1.0"
 
-fs-extra@9.0.0:
-  version "9.0.0"
-  resolved "https://registry.npmjs.org/fs-extra/-/fs-extra-9.0.0.tgz#b6afc31036e247b2466dc99c29ae797d5d4580a3"
-  integrity sha512-pmEYSk3vYsG/bF651KPUXZ+hvjpgWYw/Gc7W9NFUe3ZVLczKKWIij3IKpOrQcdw4TILtibFslZ0UmR8Vvzig4g==
-  dependencies:
-    at-least-node "^1.0.0"
-    graceful-fs "^4.2.0"
-    jsonfile "^6.0.1"
-    universalify "^1.0.0"
-
 fs-extra@9.0.1, fs-extra@^9.0.0, fs-extra@^9.0.1:
   version "9.0.1"
   resolved "https://registry.npmjs.org/fs-extra/-/fs-extra-9.0.1.tgz#910da0062437ba4c39fedd863f1675ccfefcb9fc"