From 1c9fc719dc2b74495a05f7ebc90e92e7daa03e6d Mon Sep 17 00:00:00 2001 From: Mattermost Build Date: Wed, 8 May 2024 18:12:28 +0300 Subject: [PATCH] [MM-58089] Disallow redirects to untrusted URLs without a permission prompt (#3024) (#3030) * [MM-58089] Disallow redirects to untrusted URLs without a permission prompt * Fix types * Add test (cherry picked from commit b411437a15dca7267d25e52a73fd7b654573b204) Co-authored-by: Devin Binnie <52460000+devinbinnie@users.noreply.github.com> --- i18n/en.json | 2 ++ src/main/notifications/index.ts | 2 +- src/main/permissionsManager.test.js | 17 +++++++++++++---- src/main/permissionsManager.ts | 18 +++++++++++++----- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/i18n/en.json b/i18n/en.json index f071da16..7067dd2f 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -113,9 +113,11 @@ "main.permissionsManager.checkPermission.dialog.detail.geolocation": "{appName} will use the location for setting up your timezone. You can always change this later in your computer's settings.", "main.permissionsManager.checkPermission.dialog.detail.media": "{appName} will use the microphone and camera for calls and voice messages. You can always change this later in your computer's settings.", "main.permissionsManager.checkPermission.dialog.detail.notifications": "{appName} will send you notifications for messages and calls. You can configure your notification preferences in Settings.", + "main.permissionsManager.checkPermission.dialog.detail.openExternal": "{appName} will open the requested link in an external application. If you do not trust this link, or do not recognize it, click Deny. You can always change this later in your computer's settings.", "main.permissionsManager.checkPermission.dialog.message.geolocation": "{appName} ({url}) would like to access your location.", "main.permissionsManager.checkPermission.dialog.message.media": "{appName} ({url}) would like to access the microphone and camera.", "main.permissionsManager.checkPermission.dialog.message.notifications": "{appName} ({url}) would like to send you notifications.", + "main.permissionsManager.checkPermission.dialog.message.openExternal": "{appName} ({url}) would like permission to open the following URL: {externalURL}", "main.permissionsManager.checkPermission.dialog.title": "Permission Requested", "main.tray.tray.expired": "Session Expired: Please sign in to continue receiving notifications.", "main.tray.tray.mention": "You have been mentioned", diff --git a/src/main/notifications/index.ts b/src/main/notifications/index.ts index 572db8f4..906469ce 100644 --- a/src/main/notifications/index.ts +++ b/src/main/notifications/index.ts @@ -55,7 +55,7 @@ class NotificationManager { soundName, }; - if (!await PermissionsManager.doPermissionRequest(webcontents.id, 'notifications', view.view.server.url.toString())) { + if (!await PermissionsManager.doPermissionRequest(webcontents.id, 'notifications', {requestingUrl: view.view.server.url.toString(), isMainFrame: false})) { return {status: 'not_sent', reason: 'notifications_permission_disallowed'}; } diff --git a/src/main/permissionsManager.test.js b/src/main/permissionsManager.test.js index 3643366b..09390de4 100644 --- a/src/main/permissionsManager.test.js +++ b/src/main/permissionsManager.test.js @@ -109,7 +109,7 @@ describe('main/PermissionsManager', () => { it('should allow if dialog is not required', async () => { const permissionsManager = new PermissionsManager('anyfile.json'); const cb = jest.fn(); - await permissionsManager.handlePermissionRequest({id: 2}, 'fullscreen', cb, {securityOrigin: 'http://anyurl.com'}); + await permissionsManager.handlePermissionRequest({id: 2}, 'fullscreen', cb, {requestingUrl: 'http://anyurl.com'}); expect(cb).toHaveBeenCalledWith(true); }); @@ -181,9 +181,9 @@ describe('main/PermissionsManager', () => { const cb = jest.fn(); dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0})); await Promise.all([ - permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}), - permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}), - permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {securityOrigin: 'http://anyurl.com'}), + permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}), + permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}), + permissionsManager.handlePermissionRequest({id: 2}, 'notifications', cb, {requestingUrl: 'http://anyurl.com'}), ]); expect(dialog.showMessageBox).toHaveBeenCalledTimes(1); }); @@ -203,4 +203,13 @@ describe('main/PermissionsManager', () => { await permissionsManager.handlePermissionRequest({id: 2}, 'media', cb, {securityOrigin: 'http://anyurl.com'}); expect(dialog.showMessageBox).toHaveBeenCalled(); }); + + it('should pop dialog for external applications', async () => { + const permissionsManager = new PermissionsManager('anyfile.json'); + permissionsManager.writeToFile = jest.fn(); + const cb = jest.fn(); + dialog.showMessageBox.mockReturnValue(Promise.resolve({response: 0})); + await permissionsManager.handlePermissionRequest({id: 2}, 'openExternal', cb, {requestingUrl: 'http://anyurl.com', externalURL: 'ms-excel://differenturl.com'}); + expect(dialog.showMessageBox).toHaveBeenCalled(); + }); }); diff --git a/src/main/permissionsManager.ts b/src/main/permissionsManager.ts index f1904ea7..8f10f417 100644 --- a/src/main/permissionsManager.ts +++ b/src/main/permissionsManager.ts @@ -38,6 +38,7 @@ const authorizablePermissionTypes = [ 'media', 'geolocation', 'notifications', + 'openExternal', ]; type Permissions = { @@ -67,16 +68,16 @@ export class PermissionsManager extends JsonFileManager { callback(await this.doPermissionRequest( webContents.id, permission, - details.securityOrigin ?? details.requestingUrl, + details, )); }; doPermissionRequest = async ( webContentsId: number, permission: string, - requestingURL: string, + details: PermissionRequestHandlerHandlerDetails, ) => { - log.debug('doPermissionRequest', requestingURL, permission); + log.debug('doPermissionRequest', permission, details); // is the requested permission type supported? if (!supportedPermissionTypes.includes(permission)) { @@ -89,7 +90,12 @@ export class PermissionsManager extends JsonFileManager { return true; } - const parsedURL = parseURL(requestingURL); + let url = details.requestingUrl; + if (permission === 'media' && details.securityOrigin) { + url = details.securityOrigin; + } + + const parsedURL = parseURL(url); if (!parsedURL) { return false; } @@ -142,7 +148,7 @@ export class PermissionsManager extends JsonFileManager { // Show the dialog to ask the user const {response} = await dialog.showMessageBox(mainWindow, { title: localizeMessage('main.permissionsManager.checkPermission.dialog.title', 'Permission Requested'), - message: localizeMessage(`main.permissionsManager.checkPermission.dialog.message.${permission}`, '{appName} ({url}) is requesting the "{permission}" permission.', {appName: app.name, url: parsedURL.origin, permission}), + message: localizeMessage(`main.permissionsManager.checkPermission.dialog.message.${permission}`, '{appName} ({url}) is requesting the "{permission}" permission.', {appName: app.name, url: parsedURL.origin, permission, externalURL: details.externalURL}), detail: localizeMessage(`main.permissionsManager.checkPermission.dialog.detail.${permission}`, 'Would you like to grant {appName} this permission?', {appName: app.name}), type: 'question', buttons: [ @@ -178,9 +184,11 @@ export class PermissionsManager extends JsonFileManager { t('main.permissionsManager.checkPermission.dialog.message.media'); t('main.permissionsManager.checkPermission.dialog.message.geolocation'); t('main.permissionsManager.checkPermission.dialog.message.notifications'); +t('main.permissionsManager.checkPermission.dialog.message.openExternal'); t('main.permissionsManager.checkPermission.dialog.detail.media'); t('main.permissionsManager.checkPermission.dialog.detail.geolocation'); t('main.permissionsManager.checkPermission.dialog.detail.notifications'); +t('main.permissionsManager.checkPermission.dialog.detail.openExternal'); let permissionsManager = new PermissionsManager(permissionsJson);