Skip to content

Commit

Permalink
fix: ensure guest-embedder map is updated when webview is removed
Browse files Browse the repository at this point in the history
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 Apr 30, 2020
1 parent 075472d commit b4ab9dc
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 2 deletions.
9 changes: 8 additions & 1 deletion lib/browser/guest-view-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,10 @@ const attachGuest = function (event, embedderFrameId, elementInstanceId, guestIn

// Remove an guest-embedder relationship.
const detachGuest = function (embedder, guestInstanceId) {
const guestInstance = guestInstances[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
}
});

handleMessage('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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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 b4ab9dc

Please sign in to comment.