fix: ensure guest-embedder map is updated when webview is removed #23342
Conversation
deepak1556
force-pushed
the
robo/fix_webview_desctruction_patch
branch
from
b4ab9dc
to
cde4b6e
Compare
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
deepak1556
force-pushed
the
robo/fix_webview_desctruction_patch
branch
from
cde4b6e
to
eef55a9
Compare
|
Release Notes Persisted
|
|
I was unable to backport this PR to "7-2-x" cleanly; |
|
I was unable to backport this PR to "8-x-y" cleanly; |
|
I have automatically backported this PR to "9-x-y", please check out #23374 |
…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
|
@deepak1556 has manually backported this PR to "8-x-y", please check out #23397 |
…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
|
@deepak1556 has manually backported this PR to "7-2-x", please check out #23398 |
…3342) (#23398) 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
…3342) (#23397) 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
|
thanks once more for the quick fix @deepak1556 |
|
Not sure of the timeline, but there will be one out sometime this week. |
|
@deepak1556 thanks again for the fix here. Any updates on when a new 7.2 release will be made? Looks like it is these 14 commits (which includes this one) that haven't been released as a part of 7.2 yet: v7.2.4...7-2-x |
|
This is be released in 7.3.0 https://github.com/electron/electron/commits/v7.3.0/lib/browser/guest-view-manager.js But looks like 7.2.4 is the last 7.2 release? |
|
I guess for vscode it is not relevant since it's switched to 8.3 now? |
|
vscode has reverted to Electron 7 for the upcoming release due to performance concerns with Electron 8. I have updated the version to 7.3.0 to pick up this fix. |
Description of Change
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
microsoft/vscode#96492
I was unable to isolate a test case to add for this.
Checklist
npm testpassesRelease Notes
Notes: fix crash with webview during some window management events like resize, scroll etc.