From 6e5531ae16d69087856ce7f174ba465bc759394c Mon Sep 17 00:00:00 2001
From: Stefan Malzner <stefan@adlk.io>
Date: Tue, 5 Mar 2019 16:20:40 +0100
Subject: feat(App): Add security checks for external URLs

---
 src/config.js              |  6 ++++++
 src/helpers/url-helpers.js | 15 +++++++++++++++
 src/index.js               |  6 +++++-
 src/stores/AppStore.js     | 11 ++++++++++-
 4 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 src/helpers/url-helpers.js

(limited to 'src')

diff --git a/src/config.js b/src/config.js
index a782ad667..479572edb 100644
--- a/src/config.js
+++ b/src/config.js
@@ -62,3 +62,9 @@ export const SETTINGS_PATH = path.join(app.getPath('userData'), 'config');
 
 // Replacing app.asar is not beautiful but unforunately necessary
 export const DICTIONARY_PATH = asarPath(path.join(__dirname, 'dictionaries'));
+
+export const ALLOWED_PROTOCOLS = [
+  'https:',
+  'http:',
+  'ftp:',
+];
diff --git a/src/helpers/url-helpers.js b/src/helpers/url-helpers.js
new file mode 100644
index 000000000..750d1f00c
--- /dev/null
+++ b/src/helpers/url-helpers.js
@@ -0,0 +1,15 @@
+import { URL } from 'url';
+
+import { ALLOWED_PROTOCOLS } from '../config';
+
+const debug = require('debug')('Franz:Helpers:url');
+
+export function isValidExternalURL(url) {
+  const parsedUrl = new URL(url);
+
+  const isAllowed = ALLOWED_PROTOCOLS.includes(parsedUrl.protocol);
+
+  debug('protocol check is', isAllowed, 'for:', url);
+
+  return isAllowed;
+}
diff --git a/src/index.js b/src/index.js
index 0614197a2..0e222c3d6 100644
--- a/src/index.js
+++ b/src/index.js
@@ -34,6 +34,7 @@ import {
   DEFAULT_WINDOW_OPTIONS,
 } from './config';
 import { asarPath } from './helpers/asar-helpers';
+import { isValidExternalURL } from './helpers/url-helpers';
 /* eslint-enable import/first */
 
 const debug = require('debug')('Franz:App');
@@ -294,7 +295,10 @@ const createWindow = () => {
   mainWindow.webContents.on('new-window', (e, url) => {
     debug('Open url', url);
     e.preventDefault();
-    shell.openExternal(url);
+
+    if (isValidExternalURL(url)) {
+      shell.openExternal(url);
+    }
   });
 };
 
diff --git a/src/stores/AppStore.js b/src/stores/AppStore.js
index 168aa7e48..f9009af5a 100644
--- a/src/stores/AppStore.js
+++ b/src/stores/AppStore.js
@@ -8,6 +8,7 @@ import { getDoNotDisturb } from '@meetfranz/electron-notification-state';
 import AutoLaunch from 'auto-launch';
 import prettyBytes from 'pretty-bytes';
 import ms from 'ms';
+import { URL } from 'url';
 
 import Store from './lib/Store';
 import Request from './lib/Request';
@@ -19,6 +20,7 @@ import { onVisibilityChange } from '../helpers/visibility-helper';
 import { getLocale } from '../helpers/i18n-helpers';
 
 import { getServiceIdsFromPartitions, removeServicePartitionDirectory } from '../helpers/service-helpers.js';
+import { isValidExternalURL } from '../helpers/url-helpers';
 
 const debug = require('debug')('Franz:AppStore');
 
@@ -256,7 +258,14 @@ export default class AppStore extends Store {
   }
 
   @action _openExternalUrl({ url }) {
-    shell.openExternal(url);
+    const parsedUrl = new URL(url);
+    debug('open external url', parsedUrl);
+
+    if (isValidExternalURL(url)) {
+      shell.openExternal(url);
+    }
+
+    gaEvent('External URL', 'open', parsedUrl.host);
   }
 
   @action _checkForUpdates() {
-- 
cgit v1.2.3-54-g00ecf