From 9d4093b3d8647e1896fc2c0a29c65ff46477b55b Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Sat, 29 Feb 2020 22:11:06 -0800 Subject: [PATCH] CVE-2020-10856: Enable context isolation. This fixes a vulnerability reported by Matt Austin. Signed-off-by: Anders Kaseorg --- app/renderer/js/components/webview.ts | 2 +- app/renderer/js/electron-bridge.ts | 28 +++- app/renderer/js/injected.ts | 123 ++++++++++++++++++ .../js/notification/darwin-notifications.ts | 10 +- app/renderer/js/notification/helpers.ts | 6 - app/renderer/js/notification/index.ts | 63 ++++++++- app/renderer/js/preload.ts | 93 +++++-------- app/renderer/js/utils/params-util.ts | 13 -- app/renderer/network.html | 3 - package.json | 1 + typings.d.ts | 19 --- 11 files changed, 244 insertions(+), 117 deletions(-) create mode 100644 app/renderer/js/injected.ts delete mode 100644 app/renderer/js/utils/params-util.ts diff --git a/app/renderer/js/components/webview.ts b/app/renderer/js/components/webview.ts index 3f9fa2b2b..03376d708 100644 --- a/app/renderer/js/components/webview.ts +++ b/app/renderer/js/components/webview.ts @@ -50,7 +50,7 @@ export default class WebView extends BaseComponent { ${this.props.preload ? 'preload="js/preload.js"' : ''} partition="persist:webviewsession" name="${this.props.name}" - webpreferences="javascript=yes"> + webpreferences="${this.props.nodeIntegration ? '' : 'contextIsolation, '}javascript=yes"> `; } diff --git a/app/renderer/js/electron-bridge.ts b/app/renderer/js/electron-bridge.ts index 353821431..fc14761de 100644 --- a/app/renderer/js/electron-bridge.ts +++ b/app/renderer/js/electron-bridge.ts @@ -2,6 +2,8 @@ import { ipcRenderer } from 'electron'; import { EventEmitter } from 'events'; +import { NotificationData, newNotification } from './notification'; + type ListenerType = ((...args: any[]) => void); class ElectronBridge extends EventEmitter { @@ -19,13 +21,31 @@ class ElectronBridge extends EventEmitter { this.last_active_on_system = Date.now(); } - send_event(eventName: string | symbol, ...args: any[]): void { + send_event = (eventName: string | symbol, ...args: any[]): void => { this.emit(eventName, ...args); - } + }; - on_event(eventName: string, listener: ListenerType): void { + on_event = (eventName: string, listener: ListenerType): void => { this.on(eventName, listener); - } + }; + + new_notification = ( + title: string, + options: NotificationOptions | undefined, + dispatch: (type: string, eventInit: EventInit) => boolean + ): NotificationData => + newNotification(title, options, dispatch); + + get_idle_on_system = (): boolean => this.idle_on_system; + + get_last_active_on_system = (): number => this.last_active_on_system; + + get_send_notification_reply_message_supported = (): boolean => + this.send_notification_reply_message_supported; + + set_send_notification_reply_message_supported = (value: boolean): void => { + this.send_notification_reply_message_supported = value; + }; } const electron_bridge = new ElectronBridge(); diff --git a/app/renderer/js/injected.ts b/app/renderer/js/injected.ts new file mode 100644 index 000000000..d32b2050c --- /dev/null +++ b/app/renderer/js/injected.ts @@ -0,0 +1,123 @@ +'use strict'; + +(() => { + const zulipWindow = window as typeof window & { + electron_bridge: any; + narrow: any; + page_params: any; + raw_electron_bridge: any; + }; + + const electron_bridge = { + ...zulipWindow.raw_electron_bridge, + + get idle_on_system() { + return this.get_idle_on_system(); + }, + + get last_active_on_system() { + return this.get_last_active_on_system(); + }, + + get send_notification_reply_message_supported() { + return this.get_send_notification_reply_message_supported(); + }, + + set send_notification_reply_message_supported(value: boolean) { + this.set_send_notification_reply_message_supported(value); + } + }; + + zulipWindow.electron_bridge = electron_bridge; + + (async () => { + if (document.readyState === 'loading') { + await new Promise(resolve => { + document.addEventListener('DOMContentLoaded', () => { + resolve(); + }); + }); + } + + const { page_params } = zulipWindow; + if (page_params) { + electron_bridge.send_event('zulip-loaded', { + authMethods: page_params.external_authentication_methods, + serverLanguage: page_params.default_language + }); + } + })(); + + electron_bridge.on_event('narrow-by-topic', (id: string) => { + const { narrow } = zulipWindow; + const narrowByTopic = narrow.by_topic || narrow.by_subject; + narrowByTopic(id, { trigger: 'notification' }); + }); + + function attributeListener(type: string): PropertyDescriptor { + const symbol = Symbol('on' + type); + + function listener(this: T, ev: Event): void { + if ((this as any)[symbol].call(this, ev) === false) { + ev.preventDefault(); + } + } + + return { + configurable: true, + enumerable: true, + get(this: T) { + return (this as any)[symbol]; + }, + set(this: T, value: unknown) { + if (typeof value === 'function') { + if (!(symbol in this)) { + this.addEventListener(type, listener); + } + (this as any)[symbol] = value; + } else if (symbol in this) { + this.removeEventListener(type, listener); + delete (this as any)[symbol]; + } + } + }; + } + + const NativeNotification = Notification; + + class InjectedNotification extends EventTarget { + constructor(title: string, options?: NotificationOptions) { + super(); + Object.assign( + this, + electron_bridge.new_notification(title, options, (type: string, eventInit: EventInit) => + this.dispatchEvent(new Event(type, eventInit)) + ) + ); + } + + static get maxActions(): number { + return NativeNotification.maxActions; + } + + static get permission(): NotificationPermission { + return NativeNotification.permission; + } + + static async requestPermission(callback?: NotificationPermissionCallback): Promise { + if (callback) { + callback(await Promise.resolve(NativeNotification.permission)); + } + return NativeNotification.permission; + } + } + + Object.defineProperties(InjectedNotification.prototype, { + onclick: attributeListener('click'), + onclose: attributeListener('close'), + onerror: attributeListener('error'), + onshow: attributeListener('show') + }); + + window.Notification = InjectedNotification as any; +})(); diff --git a/app/renderer/js/notification/darwin-notifications.ts b/app/renderer/js/notification/darwin-notifications.ts index 1669d5ae2..7507bc85a 100644 --- a/app/renderer/js/notification/darwin-notifications.ts +++ b/app/renderer/js/notification/darwin-notifications.ts @@ -1,17 +1,17 @@ import { ipcRenderer } from 'electron'; import { - appId, customReply, focusCurrentServer, parseReply, setupReply + appId, customReply, focusCurrentServer, parseReply } from './helpers'; import MacNotifier from 'node-mac-notifier'; import * as ConfigUtil from '../utils/config-util'; +import electron_bridge from '../electron-bridge'; type ReplyHandler = (response: string) => void; type ClickHandler = () => void; let replyHandler: ReplyHandler; let clickHandler: ClickHandler; -declare const window: ZulipWebWindow; interface NotificationHandlerArgs { response: string; } @@ -87,12 +87,12 @@ class DarwinNotification { async notificationHandler({ response }: NotificationHandlerArgs): Promise { response = await parseReply(response); focusCurrentServer(); - if (window.electron_bridge.send_notification_reply_message_supported) { - window.electron_bridge.send_event('send_notification_reply_message', this.tag, response); + if (electron_bridge.send_notification_reply_message_supported) { + electron_bridge.send_event('send_notification_reply_message', this.tag, response); return; } - setupReply(this.tag); + electron_bridge.emit('narrow-by-topic', this.tag); if (replyHandler) { replyHandler(response); return; diff --git a/app/renderer/js/notification/helpers.ts b/app/renderer/js/notification/helpers.ts index 4e3ca9048..7aa069fb5 100644 --- a/app/renderer/js/notification/helpers.ts +++ b/app/renderer/js/notification/helpers.ts @@ -134,9 +134,3 @@ export async function parseReply(reply: string): Promise { reply = reply.replace(/\\n/, '\n'); return reply; } - -export function setupReply(id: string): void { - const { narrow } = window; - const narrowByTopic = narrow.by_topic || narrow.by_subject; - narrowByTopic(id, { trigger: 'notification' }); -} diff --git a/app/renderer/js/notification/index.ts b/app/renderer/js/notification/index.ts index ed2e059ab..0f6b44c0f 100644 --- a/app/renderer/js/notification/index.ts +++ b/app/renderer/js/notification/index.ts @@ -1,5 +1,5 @@ import { remote } from 'electron'; -import * as params from '../utils/params-util'; +import electron_bridge from '../electron-bridge'; import { appId, loadBots } from './helpers'; import DefaultNotification from './default-notification'; @@ -9,15 +9,64 @@ const { app } = remote; // On windows 8 we have to explicitly set the appUserModelId otherwise notification won't work. app.setAppUserModelId(appId); -window.Notification = DefaultNotification; +let Notification = DefaultNotification; if (process.platform === 'darwin') { - window.Notification = require('./darwin-notifications'); + Notification = require('./darwin-notifications'); } -window.addEventListener('load', () => { - // eslint-disable-next-line no-undef - if (params.isPageParams() && page_params.realm_uri) { - loadBots(); +export interface NotificationData { + close(): void; + title: string; + dir: NotificationDirection; + lang: string; + body: string; + tag: string; + image: string; + icon: string; + badge: string; + vibrate: readonly number[]; + timestamp: number; + renotify: boolean; + silent: boolean; + requireInteraction: boolean; + data: unknown; + actions: readonly NotificationAction[]; +} + +export function newNotification( + title: string, + options: NotificationOptions | undefined, + dispatch: (type: string, eventInit: EventInit) => boolean +): NotificationData { + const notification = new Notification(title, options); + for (const type of ['click', 'close', 'error', 'show']) { + notification.addEventListener(type, (ev: Event) => { + if (!dispatch(type, ev)) { + ev.preventDefault(); + } + }); } + return { + close: () => notification.close(), + title: notification.title, + dir: notification.dir, + lang: notification.lang, + body: notification.body, + tag: notification.tag, + image: notification.image, + icon: notification.icon, + badge: notification.badge, + vibrate: notification.vibrate, + timestamp: notification.timestamp, + renotify: notification.renotify, + silent: notification.silent, + requireInteraction: notification.requireInteraction, + data: notification.data, + actions: notification.actions + }; +} + +electron_bridge.once('zulip-loaded', () => { + loadBots(); }); diff --git a/app/renderer/js/preload.ts b/app/renderer/js/preload.ts index 4d975df72..d4ee5ebc5 100644 --- a/app/renderer/js/preload.ts +++ b/app/renderer/js/preload.ts @@ -1,9 +1,8 @@ -import { ipcRenderer, shell } from 'electron'; +import { contextBridge, ipcRenderer, webFrame } from 'electron'; +import fs from 'fs'; import * as SetupSpellChecker from './spellchecker'; import isDev from 'electron-is-dev'; -import * as LinkUtil from './utils/link-util'; -import * as params from './utils/params-util'; import * as AuthUtil from './utils/auth-util'; import * as ConfigUtil from './utils/config-util'; @@ -16,10 +15,8 @@ import './notification'; // eslint-disable-next-line import/no-unassigned-import import './shared/preventdrag'; -declare let window: ZulipWebWindow; - import electron_bridge from './electron-bridge'; -window.electron_bridge = electron_bridge; +contextBridge.exposeInMainWorld('raw_electron_bridge', electron_bridge); ipcRenderer.on('logout', () => { // Create the menu for the below @@ -59,59 +56,33 @@ ipcRenderer.on('show-notification-settings', () => { }, 100); }); -// To prevent failing this script on linux we need to load it after the document loaded -document.addEventListener('DOMContentLoaded', (): void => { - if (params.isPageParams()) { - const authMethods = page_params.external_authentication_methods; // eslint-disable-line no-undef - const loginInApp = ConfigUtil.getConfigItem('loginInApp'); - console.log(loginInApp); - if (authMethods && !loginInApp) { - for (const authMethod of authMethods) { - const { button_id_suffix } = authMethod; - const $socialButton = document.querySelector(`button[id$="${button_id_suffix}"]`); - if ($socialButton) { - $socialButton.addEventListener('click', event => { - event.preventDefault(); - const socialLink = $socialButton.closest('form').action; - AuthUtil.openInBrowser(socialLink); - }); - } +electron_bridge.once('zulip-loaded', ({ authMethods, serverLanguage }) => { + const loginInApp = ConfigUtil.getConfigItem('loginInApp'); + console.log(loginInApp); + if (authMethods && !loginInApp) { + for (const authMethod of authMethods) { + const { button_id_suffix } = authMethod; + const $socialButton = document.querySelector(`button[id$="${button_id_suffix}"]`); + if ($socialButton) { + $socialButton.addEventListener('click', event => { + event.preventDefault(); + const socialLink = $socialButton.closest('form').action; + AuthUtil.openInBrowser(socialLink); + }); } } + } - // Get the default language of the server - const serverLanguage = page_params.default_language; // eslint-disable-line no-undef - if (serverLanguage) { - // Init spellchecker - SetupSpellChecker.init(serverLanguage); - } - // redirect users to network troubleshooting page - const getRestartButton = document.querySelector('.restart_get_events_button'); - if (getRestartButton) { - getRestartButton.addEventListener('click', () => { - ipcRenderer.send('forward-message', 'reload-viewer'); - }); - } - // Open image attachment link in the lightbox instead of opening in the default browser - const { $, lightbox } = window; - $('#main_div').on('click', '.message_content p a', function (this: HTMLElement, event: Event) { - const url = $(this).attr('href'); - - if (LinkUtil.isImage(url)) { - const $img = $(this).parent().siblings('.message_inline_image').find('img'); - - // prevent the image link from opening in a new page. - event.preventDefault(); - // prevent the message compose dialog from happening. - event.stopPropagation(); - - // Open image in the default browser if image preview is unavailable - if (!$img[0]) { - shell.openExternal(window.location.origin + url); - } - // Open image in lightbox - lightbox.open($img); - } + // Get the default language of the server + if (serverLanguage) { + // Init spellchecker + SetupSpellChecker.init(serverLanguage); + } + // redirect users to network troubleshooting page + const getRestartButton = document.querySelector('.restart_get_events_button'); + if (getRestartButton) { + getRestartButton.addEventListener('click', () => { + ipcRenderer.send('forward-message', 'reload-viewer'); }); } }); @@ -152,8 +123,8 @@ ipcRenderer.on('set-active', () => { if (isDev) { console.log('active'); } - window.electron_bridge.idle_on_system = false; - window.electron_bridge.last_active_on_system = Date.now(); + electron_bridge.idle_on_system = false; + electron_bridge.last_active_on_system = Date.now(); }); // Set user as idle and time of last activity is left unchanged @@ -161,5 +132,9 @@ ipcRenderer.on('set-idle', () => { if (isDev) { console.log('idle'); } - window.electron_bridge.idle_on_system = true; + electron_bridge.idle_on_system = true; }); + +webFrame.executeJavaScript( + fs.readFileSync(require.resolve('./injected'), 'utf8') +); diff --git a/app/renderer/js/utils/params-util.ts b/app/renderer/js/utils/params-util.ts deleted file mode 100644 index 78d70f2cf..000000000 --- a/app/renderer/js/utils/params-util.ts +++ /dev/null @@ -1,13 +0,0 @@ -/* eslint-disable unicorn/prevent-abbreviations */ - -// This util function returns the page params if they're present else returns null -export function isPageParams(): null | object { - let webpageParams = null; - try { - // eslint-disable-next-line no-undef - webpageParams = page_params; - } catch (_) { - webpageParams = null; - } - return webpageParams; -} diff --git a/app/renderer/network.html b/app/renderer/network.html index 7e2a2fe09..15782683d 100644 --- a/app/renderer/network.html +++ b/app/renderer/network.html @@ -23,7 +23,4 @@ - - - diff --git a/package.json b/package.json index c615031fc..80ed1b069 100644 --- a/package.json +++ b/package.json @@ -245,6 +245,7 @@ "overrides": [ { "files": [ + "app/renderer/js/injected.ts", "gulpfile.js", "scripts/notarize.js", "tests/**/*.js", diff --git a/typings.d.ts b/typings.d.ts index 90ee9ba04..eab2bea94 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -1,22 +1,3 @@ declare module '@electron-elements/send-feedback'; declare module 'node-mac-notifier'; declare module 'wurl'; - -interface PageParamsObject { - realm_uri: string; - default_language: string; - external_authentication_methods: any; -} -// eslint-disable-next-line unicorn/prevent-abbreviations -declare let page_params: PageParamsObject; - -// This is mostly zulip side of code we access from window -interface Window { - $: any; - narrow: any; -} - -interface ZulipWebWindow extends Window { - electron_bridge: any; - lightbox: any; -}