Skip to content

Commit

Permalink
CVE-2020-10856: Enable context isolation.
Browse files Browse the repository at this point in the history
This fixes a vulnerability reported by Matt Austin.

Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
  • Loading branch information
andersk committed Mar 31, 2020
1 parent 20a6c5d commit 9d4093b
Show file tree
Hide file tree
Showing 11 changed files with 244 additions and 117 deletions.
2 changes: 1 addition & 1 deletion app/renderer/js/components/webview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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">
</webview>`;
}

Expand Down
28 changes: 24 additions & 4 deletions app/renderer/js/electron-bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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();
Expand Down
123 changes: 123 additions & 0 deletions app/renderer/js/injected.ts
Original file line number Diff line number Diff line change
@@ -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<T extends EventTarget>(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<NotificationPermission> {
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;
})();
10 changes: 5 additions & 5 deletions app/renderer/js/notification/darwin-notifications.ts
Original file line number Diff line number Diff line change
@@ -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;
}
Expand Down Expand Up @@ -87,12 +87,12 @@ class DarwinNotification {
async notificationHandler({ response }: NotificationHandlerArgs): Promise<void> {
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;
Expand Down
6 changes: 0 additions & 6 deletions app/renderer/js/notification/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,3 @@ export async function parseReply(reply: string): Promise<string> {
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' });
}
63 changes: 56 additions & 7 deletions app/renderer/js/notification/index.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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();
});
Loading

3 comments on commit 9d4093b

@abhigyank
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @andersk Can you give a brief explanation of the changes and requirement of this commit?

@andersk
Copy link
Member Author

@andersk andersk commented on 9d4093b Apr 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abhigyank
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understood thanks!
Really amazing work on this 🎉

Please sign in to comment.