Skip to content

Commit

Permalink
fix: ensure guest-embedder map is updated when webview is removed (#2…
Browse files Browse the repository at this point in the history
…3342)

There are use cases of webview where the container holding the webview is not
actually destroyed first, instead just webview gets removed from DOM, in such
situations the browser process map is not updated accordingly and holds reference
to stale guest contents, and any window operations like scroll, resize or keyboard
events that has to chain through browser embedder will lead to UAF crash.

Ref: microsoft/vscode#92420
  • Loading branch information
deepak1556 committed May 1, 2020
1 parent 4bbb2fb commit c438b93
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
7 changes: 7 additions & 0 deletions lib/browser/guest-view-manager.js
Expand Up @@ -251,6 +251,9 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn
// Remove an guest-embedder relationship.
const detachGuest = function (embedder, guestInstanceId) {
const guestInstance = guestInstances[guestInstanceId];

if (!guestInstance) return;

if (embedder !== guestInstance.embedder) {
return;
}
Expand Down Expand Up @@ -341,6 +344,10 @@ handleMessage('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', function (event, embed
}
});

handleMessageSync('ELECTRON_GUEST_VIEW_MANAGER_DETACH_GUEST', function (event, guestInstanceId) {
return detachGuest(event.sender, guestInstanceId);
});

// this message is sent by the actual <webview>
ipcMainInternal.on('ELECTRON_GUEST_VIEW_MANAGER_FOCUS_CHANGE', function (event, focus, guestInstanceId) {
const guest = getGuest(guestInstanceId);
Expand Down
8 changes: 7 additions & 1 deletion lib/renderer/web-view/guest-view-internal.ts
@@ -1,5 +1,6 @@
import { webFrame, IpcMessageEvent } from 'electron';
import { ipcRendererInternal } from '@electron/internal/renderer/ipc-renderer-internal';
import * as ipcRendererUtils from '@electron/internal/renderer/ipc-renderer-internal-utils';

import { WebViewImpl } from '@electron/internal/renderer/web-view/web-view-impl';

Expand Down Expand Up @@ -105,8 +106,13 @@ export function attachGuest (
ipcRendererInternal.invoke('ELECTRON_GUEST_VIEW_MANAGER_ATTACH_GUEST', embedderFrameId, elementInstanceId, guestInstanceId, params);
}

export function detachGuest (guestInstanceId: number) {
return ipcRendererUtils.invokeSync('ELECTRON_GUEST_VIEW_MANAGER_DETACH_GUEST', guestInstanceId);
}

export const guestViewInternalModule = {
deregisterEvents,
createGuest,
attachGuest
attachGuest,
detachGuest
};
3 changes: 3 additions & 0 deletions lib/renderer/web-view/web-view-element.ts
Expand Up @@ -66,6 +66,9 @@ const defineWebViewElement = (v8Util: NodeJS.V8UtilBinding, webViewImpl: typeof
return;
}
guestViewInternal.deregisterEvents(internal.viewInstanceId);
if (internal.guestInstanceId) {
guestViewInternal.detachGuest(internal.guestInstanceId);
}
internal.elementAttached = false;
this.internalInstanceId = 0;
internal.reset();
Expand Down

0 comments on commit c438b93

Please sign in to comment.