From a52e96694dd5b2be836c5b260e234810273f5c3e Mon Sep 17 00:00:00 2001 From: Claudio Costa Date: Thu, 2 Mar 2023 12:10:47 -0600 Subject: [PATCH] [MM-50973] Harden Calls windows URL navigation checks (#2573) * Harden Calls windows URL navigation checks * Update src/main/windows/callsWidgetWindow.ts Co-authored-by: Eva Sarafianou * Fix bad merge * Extract team name to use isUrlType * Simplify --------- Co-authored-by: Eva Sarafianou --- src/common/utils/url.test.js | 54 ++++++++++++++++++++++ src/common/utils/url.ts | 31 ++++++++++--- src/main/views/viewManager.ts | 2 +- src/main/views/webContentEvents.ts | 11 +++-- src/main/windows/callsWidgetWindow.test.js | 39 +++++++++++++--- src/main/windows/callsWidgetWindow.ts | 40 +++++++++++----- src/main/windows/windowManager.ts | 2 +- 7 files changed, 148 insertions(+), 31 deletions(-) diff --git a/src/common/utils/url.test.js b/src/common/utils/url.test.js index 371e9959..8b36fbfe 100644 --- a/src/common/utils/url.test.js +++ b/src/common/utils/url.test.js @@ -275,4 +275,58 @@ describe('common/utils/url', () => { )).toBe(false); }); }); + + describe('isCallsPopOutURL', () => { + it('should match correct URL', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.org', + 'http://example.org/team/com.mattermost.calls/expanded/callid', + 'callid', + )).toBe(true); + }); + + it('should match with subpath', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.org/subpath', + 'http://example.org/subpath/team/com.mattermost.calls/expanded/callid', + 'callid', + )).toBe(true); + }); + + it('should match with teamname with dash', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.org', + 'http://example.org/team-name/com.mattermost.calls/expanded/callid', + 'callid', + )).toBe(true); + }); + + it('should not match with invalid team name', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.org', + 'http://example.org/invalid$team/com.mattermost.calls/expanded/othercallid', + 'callid', + )).toBe(false); + }); + + it('should not match with incorrect callid', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.org', + 'http://example.org/team/com.mattermost.calls/expanded/othercallid', + 'callid', + )).toBe(false); + }); + + it('should not match with incorrect origin', () => { + expect(urlUtils.isCallsPopOutURL( + 'http://example.com', + 'http://example.org/team/com.mattermost.calls/expanded/callid', + 'callid', + )).toBe(false); + }); + + it('should not match with missing arguments', () => { + expect(urlUtils.isCallsPopOutURL()).toBe(false); + }); + }); }); diff --git a/src/common/utils/url.ts b/src/common/utils/url.ts index 686f4991..678f14d8 100644 --- a/src/common/utils/url.ts +++ b/src/common/utils/url.ts @@ -4,7 +4,7 @@ import {isHttpsUri, isHttpUri, isUri} from 'valid-url'; import buildConfig from 'common/config/buildConfig'; -import {customLoginRegexPaths, nonTeamUrlPaths} from 'common/utils/constants'; +import {customLoginRegexPaths, nonTeamUrlPaths, CALLS_PLUGIN_ID} from 'common/utils/constants'; function isValidURL(testURL: string) { return Boolean(isHttpUri(testURL) || isHttpsUri(testURL)) && Boolean(parseURL(testURL)); @@ -98,12 +98,6 @@ function isAdminUrl(serverUrl: URL | string, inputURL: URL | string) { } function isTeamUrl(serverUrl: URL | string, inputURL: URL | string, withApi?: boolean) { - const parsedURL = parseURL(inputURL); - const server = getServerInfo(serverUrl); - if (!parsedURL || !server || (!equalUrlsIgnoringSubpath(server.url, parsedURL))) { - return false; - } - const paths = [...getManagedResources(), ...nonTeamUrlPaths]; if (withApi) { @@ -184,6 +178,28 @@ function cleanPathName(basePathName: string, pathName: string) { return pathName; } +function isCallsPopOutURL(serverURL: URL | string, inputURL: URL | string, callID: string) { + if (!serverURL || !inputURL || !callID) { + return false; + } + + const parsedURL = parseURL(inputURL); + const server = getServerInfo(serverURL); + if (!server || !parsedURL) { + return false; + } + + const matches = parsedURL.pathname.match(new RegExp(`^${server.subpath}([A-Za-z0-9-_]+)/`, 'i')); + if (matches?.length !== 2) { + return false; + } + + const teamName = matches[1]; + const subPath = `${teamName}/${CALLS_PLUGIN_ID}/expanded/${callID}`; + + return isUrlType(subPath, serverURL, inputURL); +} + export default { isValidURL, isValidURI, @@ -201,4 +217,5 @@ export default { isUrlType, cleanPathName, startsWithProtocol, + isCallsPopOutURL, }; diff --git a/src/main/views/viewManager.ts b/src/main/views/viewManager.ts index 99614394..34b59a97 100644 --- a/src/main/views/viewManager.ts +++ b/src/main/views/viewManager.ts @@ -284,7 +284,7 @@ export class ViewManager { log.error(`Couldn't find a view with the name ${viewName}`); return; } - WebContentsEventManager.addMattermostViewEventListeners(view, this.getServers); + WebContentsEventManager.addMattermostViewEventListeners(view); } finishLoading = (server: string) => { diff --git a/src/main/views/webContentEvents.ts b/src/main/views/webContentEvents.ts index e37293a3..e9755407 100644 --- a/src/main/views/webContentEvents.ts +++ b/src/main/views/webContentEvents.ts @@ -4,8 +4,6 @@ import {BrowserWindow, session, shell, WebContents} from 'electron'; import log from 'electron-log'; -import {TeamWithTabs} from 'types/config'; - import Config from 'common/config'; import urlUtils from 'common/utils/url'; @@ -74,6 +72,11 @@ export class WebContentsEventManager { return; } + const callID = WindowManager.callsWidgetWindow?.getCallID(); + if (serverURL && callID && urlUtils.isCallsPopOutURL(serverURL, parsedURL, callID)) { + return; + } + log.info(`Prevented desktop from navigating to: ${url}`); event.preventDefault(); }; @@ -221,10 +224,9 @@ export class WebContentsEventManager { } }; - addMattermostViewEventListeners = (mmview: MattermostView, getServersFunction: () => TeamWithTabs[]) => { + addMattermostViewEventListeners = (mmview: MattermostView) => { this.addWebContentsEventListeners( mmview.view.webContents, - getServersFunction, (contents: WebContents) => { contents.on('page-title-updated', mmview.handleTitleUpdate); contents.on('page-favicon-updated', mmview.handleFaviconUpdate); @@ -242,7 +244,6 @@ export class WebContentsEventManager { addWebContentsEventListeners = ( contents: WebContents, - getServersFunction: () => TeamWithTabs[], addListeners?: (contents: WebContents) => void, removeListeners?: (contents: WebContents) => void, ) => { diff --git a/src/main/windows/callsWidgetWindow.test.js b/src/main/windows/callsWidgetWindow.test.js index d2bcc1c0..35f75fc5 100644 --- a/src/main/windows/callsWidgetWindow.test.js +++ b/src/main/windows/callsWidgetWindow.test.js @@ -27,7 +27,7 @@ jest.mock('electron', () => ({ })); jest.mock('../views/webContentEvents', () => ({ - generateNewWindowListener: jest.fn(), + addWebContentsEventListeners: jest.fn(), })); jest.mock('common/utils/url', () => { @@ -413,8 +413,15 @@ describe('main/windows/callsWidgetWindow', () => { it('menubar disabled on popout', () => { const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig); - expect(widgetWindow.onPopOutOpen()).toHaveProperty('action', 'allow'); - expect(widgetWindow.onPopOutOpen().overrideBrowserWindowOptions).toHaveProperty('autoHideMenuBar', true); + const popOutURL = 'http://localhost:8065/team/com.mattermost.calls/expanded/test-call-id'; + expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'allow'); + expect(widgetWindow.onPopOutOpen({url: popOutURL}).overrideBrowserWindowOptions).toHaveProperty('autoHideMenuBar', true); + }); + + it('wrong popout url disabled', () => { + const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig); + const popOutURL = 'http://localhost/team/com.mattermost.calls/expanded/test-call-id'; + expect(widgetWindow.onPopOutOpen({url: popOutURL})).toHaveProperty('action', 'deny'); }); it('onPopOutFocus', () => { @@ -434,7 +441,6 @@ describe('main/windows/callsWidgetWindow', () => { const popOut = new EventEmitter(); popOut.webContents = { - setWindowOpenHandler: jest.fn(), on: jest.fn(), id: 'webContentsId', }; @@ -448,8 +454,7 @@ describe('main/windows/callsWidgetWindow', () => { widgetWindow.onPopOutCreate(popOut); expect(widgetWindow.popOut).toBe(popOut); - expect(popOut.webContents.setWindowOpenHandler).toHaveBeenCalled(); - expect(WebContentsEventManager.generateNewWindowListener).toHaveBeenCalledWith('webContentsId', true); + expect(WebContentsEventManager.addWebContentsEventListeners).toHaveBeenCalledWith(popOut.webContents); widgetWindow.onPopOutFocus(); expect(popOut.focus).toHaveBeenCalled(); @@ -508,5 +513,27 @@ describe('main/windows/callsWidgetWindow', () => { sender: baseWindow.webContents, })).toEqual(true); }); + + it('getPopOutWebContentsId', () => { + const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig); + widgetWindow.popOut = { + webContents: { + id: 'popOutID', + }, + }; + expect(widgetWindow.getPopOutWebContentsId()).toBe('popOutID'); + }); + + it('onNavigate', () => { + const widgetWindow = new CallsWidgetWindow(mainWindow, mainView, widgetConfig); + + const ev = {preventDefault: jest.fn()}; + + widgetWindow.onNavigate(ev, widgetWindow.getWidgetURL()); + expect(ev.preventDefault).not.toHaveBeenCalled(); + + widgetWindow.onNavigate(ev, 'http://localhost:8065/invalid/url'); + expect(ev.preventDefault).toHaveBeenCalledTimes(1); + }); }); }); diff --git a/src/main/windows/callsWidgetWindow.ts b/src/main/windows/callsWidgetWindow.ts index 6da8e1ab..c02fff99 100644 --- a/src/main/windows/callsWidgetWindow.ts +++ b/src/main/windows/callsWidgetWindow.ts @@ -30,7 +30,6 @@ import { CALLS_WIDGET_SHARE_SCREEN, } from 'common/communication'; import webContentsEventManager from 'main/views/webContentEvents'; -import Config from 'common/config'; type LoadURLOpts = { extraHeaders: string; @@ -82,6 +81,10 @@ export default class CallsWidgetWindow extends EventEmitter { this.win.webContents.setWindowOpenHandler(this.onPopOutOpen); this.win.webContents.on('did-create-window', this.onPopOutCreate); + // Calls widget window is not supposed to navigate anywhere else. + this.win.webContents.on('will-navigate', this.onNavigate); + this.win.webContents.on('did-start-navigation', this.onNavigate); + this.load(); } @@ -102,6 +105,14 @@ export default class CallsWidgetWindow extends EventEmitter { return this.config.callID; } + private onNavigate = (ev: Event, url: string) => { + if (url === this.getWidgetURL()) { + return; + } + log.warn(`CallsWidgetWindow: prevented widget window from navigating to: ${url}`); + ev.preventDefault(); + } + private load() { const opts = {} as LoadURLOpts; this.win.loadURL(this.getWidgetURL(), opts).catch((reason) => { @@ -209,22 +220,25 @@ export default class CallsWidgetWindow extends EventEmitter { this.setBounds(initialBounds); } - private onPopOutOpen = () => { - return { - action: 'allow' as const, - overrideBrowserWindowOptions: { - autoHideMenuBar: true, - }, - }; + private onPopOutOpen = ({url}: {url: string}) => { + if (urlUtils.isCallsPopOutURL(this.mainView.serverInfo.server.url, url, this.config.callID)) { + return { + action: 'allow' as const, + overrideBrowserWindowOptions: { + autoHideMenuBar: true, + }, + }; + } + + log.warn(`CallsWidgetWindow.onPopOutOpen: prevented window open to ${url}`); + return {action: 'deny' as const}; } private onPopOutCreate = (win: BrowserWindow) => { this.popOut = win; // Let the webContentsEventManager handle links that try to open a new window - const spellcheck = Config.useSpellChecker; - const newWindow = webContentsEventManager.generateNewWindowListener(this.popOut.webContents.id, spellcheck); - this.popOut.webContents.setWindowOpenHandler(newWindow); + webContentsEventManager.addWebContentsEventListeners(this.popOut.webContents); } private onPopOutFocus = () => { @@ -241,6 +255,10 @@ export default class CallsWidgetWindow extends EventEmitter { return this.win.webContents.id; } + public getPopOutWebContentsId() { + return this.popOut?.webContents.id; + } + public getURL() { return urlUtils.parseURL(this.win.webContents.getURL()); } diff --git a/src/main/windows/windowManager.ts b/src/main/windows/windowManager.ts index fea72e23..c8a044e4 100644 --- a/src/main/windows/windowManager.ts +++ b/src/main/windows/windowManager.ts @@ -947,7 +947,7 @@ export class WindowManager { } getServerURLFromWebContentsId = (id: number) => { - if (this.callsWidgetWindow && id === this.callsWidgetWindow.getWebContentsId()) { + if (this.callsWidgetWindow && (id === this.callsWidgetWindow.getWebContentsId() || id === this.callsWidgetWindow.getPopOutWebContentsId())) { return this.callsWidgetWindow.getURL(); }