From 7c5e88f8b0214a2316667530842b03f27037b984 Mon Sep 17 00:00:00 2001 From: Lynn Neir Date: Sun, 12 Feb 2017 16:57:56 -0800 Subject: [PATCH 1/2] add security check for api calls --- js/child-preload.js | 2 - js/main-preload.js | 2 - js/main.js | 178 ++++++++++++++++++++++++++++---------------- 3 files changed, 114 insertions(+), 68 deletions(-) diff --git a/js/child-preload.js b/js/child-preload.js index 0d9632f6..90228884 100644 --- a/js/child-preload.js +++ b/js/child-preload.js @@ -5,8 +5,6 @@ // to leak some node module into: // https://medium.com/@leonli/securing-embedded-external-content-in-electron-node-js-8b6ef665cd8e#.fex4e68p7 // https://slack.engineering/building-hybrid-applications-with-electron-dc67686de5fb#.tp6zz1nrk -// as suggested above: consider injecting key into window that can be used to -// validate operations. // // also to bring pieces of node.js: // https://github.com/electron/electron/issues/2984 diff --git a/js/main-preload.js b/js/main-preload.js index 96439ad0..6e12179d 100644 --- a/js/main-preload.js +++ b/js/main-preload.js @@ -5,8 +5,6 @@ // to leak some node module into: // https://medium.com/@leonli/securing-embedded-external-content-in-electron-node-js-8b6ef665cd8e#.fex4e68p7 // https://slack.engineering/building-hybrid-applications-with-electron-dc67686de5fb#.tp6zz1nrk -// as suggested above: consider injecting key into window that can be used to -// validate operations. // // also to bring pieces of node.js: // https://github.com/electron/electron/issues/2984 diff --git a/js/main.js b/js/main.js index bd06831c..348ce417 100644 --- a/js/main.js +++ b/js/main.js @@ -2,92 +2,142 @@ const electron = require('electron'); const packageJSON = require('../package.json'); const menuTemplate = require('./menuTemplate.js'); const path = require('path'); - const app = electron.app -const BrowserWindow = electron.BrowserWindow; + +// Keep a global reference of the window object, if you don't, the window will +// be closed automatically when the JavaScript object is garbage collected. +let mainWindow; +let windows = {}; let willQuitApp = false; -if (require('electron-squirrel-startup')) return; +if (require('electron-squirrel-startup')) { + return; +} if (isDevEnv()) { // needed for development env because local server doesn't have cert app.commandLine.appendSwitch('--ignore-certificate-errors'); } -// Keep a global reference of the window object, if you don't, the window will -// be closed automatically when the JavaScript object is garbage collected. -let mainWindow; -let childWindows = []; - function isDevEnv() { - var isDev = process.env.ELECTRON_DEV ? + let isDev = process.env.ELECTRON_DEV ? process.env.ELECTRON_DEV.trim().toLowerCase() === "true" : false; return isDev; } function createMainWindow () { - // note: for now, turning off node integration as this is causing failure with - // onelogin, jquery can not get initialized. electron's node integration - // conflicts on the window object. - mainWindow = new BrowserWindow({ - title: 'Symphony', - width: 1024, height: 768, - webPreferences: { - sandbox: false, - nodeIntegration: false, - preload: path.join(__dirname, '/main-preload.js') - } - }); + let key = getWindowKey(); - mainWindow.loadURL(packageJSON.homepage); - - const menu = electron.Menu.buildFromTemplate(menuTemplate(app)); - electron.Menu.setApplicationMenu(menu); + // note: for now, turning off node integration as this is causing failure with + // onelogin, jquery can not get initialized. electron's node integration + // conflicts on the window object. + mainWindow = new electron.BrowserWindow({ + title: 'Symphony', + width: 1024, height: 768, + webPreferences: { + sandbox: false, + nodeIntegration: false, + preload: path.join(__dirname, '/main-preload.js'), + winKey: key + } + }); - mainWindow.on('close', function(e) { - if (willQuitApp) { + storeWindowKey(key, mainWindow) + + mainWindow.loadURL(packageJSON.homepage); + + const menu = electron.Menu.buildFromTemplate(menuTemplate(app)); + electron.Menu.setApplicationMenu(menu); + + mainWindow.on('close', function(e) { + if (willQuitApp) { + mainWindow = null; + return; + } + // mac should hide window when hitting x close + if (process.platform === 'darwin') { + mainWindow.hide(); + e.preventDefault(); + } + }); + + mainWindow.on('closed', function () { + // Dereference the window object, usually you would store windows + // in an array if your app supports multi windows, this is the time + // when you should delete the corresponding element. mainWindow = null; - return; - } - // mac should hide window when hitting x close - if (process.platform === 'darwin') { - mainWindow.hide(); - e.preventDefault(); - } - }); - mainWindow.on('closed', function () { - // Dereference the window object, usually you would store windows - // in an array if your app supports multi windows, this is the time - // when you should delete the corresponding element. - mainWindow = null; - }); + }); - // open external links in default browser - window.open - mainWindow.webContents.on('new-window', function(event, url) { - event.preventDefault(); - electron.shell.openExternal(url); - }); + // open external links in default browser - window.open + mainWindow.webContents.on('new-window', function(event, url) { + event.preventDefault(); + electron.shell.openExternal(url); + }); +} + +function getWindowKey() { + // generate guid: + // http://stackoverflow.com/questions/105034/create-guid-uuid-in-javascript + function s4() { + return Math.floor((1 + Math.random()) * 0x10000).toString(16) + .substring(1); + } + return s4() + s4() + '-' + s4() + '-' + s4() + '-' + + s4() + '-' + s4() + s4() + s4(); +} + +function storeWindowKey(key, browserWin) { + windows[key] = browserWin; +} + +/** + * Ensure events comes from a window that we have created. + * @param {EventEmitter} event node emitter event to be tested + * @return {Boolean} returns true if exists otherwise false + */ +function isValidWindow(event) { + if (event && event.sender) { + // validate that event sender is from window we created + let browserWin = electron.BrowserWindow.fromWebContents(event.sender) + let winKey = event.sender.browserWindowOptions && + event.sender.browserWindowOptions.webPreferences && + event.sender.browserWindowOptions.webPreferences.winKey; + + if (browserWin instanceof electron.BrowserWindow) { + let win = windows[winKey]; + return win && win === browserWin; + } + } + + return false; } electron.ipcMain.on('symphony-msg', (event, arg) => { - if (arg && arg.cmd === 'open' && arg.url) { - var width = arg.width || 1024; - var height = arg.height || 768; - var title = arg.title || 'Symphony'; + if (!isValidWindow(event)) { + console.log('invalid window try to perform action, ignoring action.'); + return; + } - let childWindow = new BrowserWindow({ + if (arg && arg.cmd === 'open' && arg.url) { + let width = arg.width || 1024; + let height = arg.height || 768; + let title = arg.title || 'Symphony'; + let winKey = getWindowKey(); + + let childWindow = new electron.BrowserWindow({ title: title, width: width, height: height, webPreferences: { sandbox: false, nodeIntegration: false, - preload: path.join(__dirname, '/child-preload.js') + preload: path.join(__dirname, '/child-preload.js'), + winKey: winKey } }); - childWindows.push(childWindow); + storeWindowKey(winKey, childWindow); childWindow.loadURL(arg.url); return; } @@ -105,17 +155,17 @@ app.on('before-quit', function() { }); app.on('window-all-closed', function () { - // On OS X it is common for applications and their menu bar - // to stay active until the user quits explicitly with Cmd + Q - if (process.platform !== 'darwin') { - app.quit(); - } -}) + // On OS X it is common for applications and their menu bar + // to stay active until the user quits explicitly with Cmd + Q + if (process.platform !== 'darwin') { + app.quit(); + } +}); app.on('activate', function () { - if (mainWindow === null) { - createMainWindow(); - } else { - mainWindow.show(); - } + if (mainWindow === null) { + createMainWindow(); + } else { + mainWindow.show(); + } }); From fee036f98fb97b0020c1db0a552e15398010cf2e Mon Sep 17 00:00:00 2001 From: Lynn Neir Date: Sun, 12 Feb 2017 17:11:02 -0800 Subject: [PATCH 2/2] turn on sandbox --- js/main.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/js/main.js b/js/main.js index 348ce417..ccaf9bdd 100644 --- a/js/main.js +++ b/js/main.js @@ -36,7 +36,7 @@ function createMainWindow () { title: 'Symphony', width: 1024, height: 768, webPreferences: { - sandbox: false, + sandbox: true, nodeIntegration: false, preload: path.join(__dirname, '/main-preload.js'), winKey: key @@ -46,7 +46,7 @@ function createMainWindow () { storeWindowKey(key, mainWindow) mainWindow.loadURL(packageJSON.homepage); - + const menu = electron.Menu.buildFromTemplate(menuTemplate(app)); electron.Menu.setApplicationMenu(menu); @@ -130,7 +130,7 @@ electron.ipcMain.on('symphony-msg', (event, arg) => { width: width, height: height, webPreferences: { - sandbox: false, + sandbox: true, nodeIntegration: false, preload: path.join(__dirname, '/child-preload.js'), winKey: winKey