diff --git a/app/browser/menu.js b/app/browser/menu.js index 0e3407fd679..1b87a96985a 100644 --- a/app/browser/menu.js +++ b/app/browser/menu.js @@ -63,7 +63,7 @@ const createFileSubmenu = () => { appActions.createTabRequested({ url: fileUrl(path), windowId: focusedWindow.id - }) + }, false, false, false, true) }) } }) diff --git a/app/browser/reducers/tabsReducer.js b/app/browser/reducers/tabsReducer.js index 06653904cb5..c5d9ae735ca 100644 --- a/app/browser/reducers/tabsReducer.js +++ b/app/browser/reducers/tabsReducer.js @@ -37,6 +37,7 @@ const {getFlashResourceId} = require('../../../js/flash') const {l10nErrorText} = require('../../common/lib/httpUtil') const flash = require('../../../js/flash') const {isSourceAboutUrl, isTargetAboutUrl, isNavigatableAboutPage} = require('../../../js/lib/appUrlUtil') +const {isFileScheme} = require('../../../js/lib/urlutil') const {shouldDebugTabEvents} = require('../../cmdLine') const getWebRTCPolicy = (state, tabId) => { @@ -249,6 +250,10 @@ const tabsReducer = (state, action, immutableAction) => { windows.focus(windowId) } const url = action.getIn(['createProperties', 'url']) + if (isFileScheme(url) && !action.get('allowFile')) { + // Don't allow 'open in new tab' to open file:// URLs for security + action = action.setIn(['createProperties', 'url'], 'about:blank') + } setImmediate(() => { if (action.get('activateIfOpen') || ((isSourceAboutUrl(url) || isTargetAboutUrl(url)) && isNavigatableAboutPage(url))) { diff --git a/app/browser/reducers/windowsReducer.js b/app/browser/reducers/windowsReducer.js index 73abc4e31eb..0d7232ca2c5 100644 --- a/app/browser/reducers/windowsReducer.js +++ b/app/browser/reducers/windowsReducer.js @@ -22,6 +22,7 @@ const {makeImmutable, isImmutable} = require('../../common/state/immutableUtil') const electron = require('electron') const BrowserWindow = electron.BrowserWindow const firstDefinedValue = require('../../../js/lib/functional').firstDefinedValue +const {isFileScheme} = require('../../../js/lib/urlutil') const settings = require('../../../js/constants/settings') const getSetting = require('../../../js/settings').getSetting @@ -266,6 +267,11 @@ const handleCreateWindowAction = (state, action = Immutable.Map()) => { if (Array.isArray(frameOpts)) { frames = frameOpts } else { + // Don't allow 'open in new window' to open a file:// URL for + // security reasons + if (isFileScheme(frameOpts.location)) { + frameOpts.location = 'about:blank' + } frames = [ frameOpts ] } } else { diff --git a/js/actions/appActions.js b/js/actions/appActions.js index 6ae4bc9cd1a..390c11ffa9a 100644 --- a/js/actions/appActions.js +++ b/js/actions/appActions.js @@ -268,13 +268,16 @@ const appActions = { * @param {Boolean} activateIfOpen if the tab is already open with the same properties, * switch to it instead of creating a new one * @param {Boolean} isRestore when true, won't try to activate the new tab, even if the user preference indicates to + * @param {Boolean} focusWindow + * @param {Boolean} allowFile - When true, allows file:// URLs to be opened */ - createTabRequested: function (createProperties, activateIfOpen = false, isRestore = false, focusWindow = false) { + createTabRequested: function (createProperties, activateIfOpen = false, isRestore = false, focusWindow = false, allowFile = false) { dispatch({ actionType: appConstants.APP_CREATE_TAB_REQUESTED, createProperties, activateIfOpen, isRestore, + allowFile, focusWindow }) }, diff --git a/js/lib/urlutil.js b/js/lib/urlutil.js index 81058a3eb40..50b422d6603 100644 --- a/js/lib/urlutil.js +++ b/js/lib/urlutil.js @@ -424,7 +424,10 @@ const UrlUtil = { * @return {boolean} */ isFileScheme: function (url) { - return this.getScheme(url) === fileScheme + if (!url) { + return false + } + return urlParse(url).protocol === 'file:' }, /** diff --git a/test/unit/lib/urlutilTestComponents.js b/test/unit/lib/urlutilTestComponents.js index 985efc62a26..6627ac9ce01 100644 --- a/test/unit/lib/urlutilTestComponents.js +++ b/test/unit/lib/urlutilTestComponents.js @@ -329,6 +329,11 @@ module.exports = { } }, 'returns false when input:': { + 'is falsey': (test) => { + test.equal(urlUtil().isFileScheme(''), false) + test.equal(urlUtil().isFileScheme(), false) + test.equal(urlUtil().isFileScheme(null), false) + }, 'is an absolute file path without scheme': (test) => { test.equal(urlUtil().isFileScheme('/file/path/to/file'), false) },