#53242: fix: avoid UB in menu.popup with frame from OSR window on macOS
Merged
Description of Change
Refactors this code to avoid an unconditional static_cast which would cast to the wrong type in the case of an offscreen window.
UBSan Output
../../electron/shell/browser/api/electron_api_menu_mac.mm:184:11: runtime error: downcast of address 0x012c0d942d80 which does not point to an object of type 'content::RenderWidgetHostViewMac'
0x012c0d942d80: note: object is of type 'electron::OffScreenRenderWidgetHostView'
97 93 89 cd b0 09 56 25 03 00 00 00 18 0f 56 25 03 00 00 00 00 ab ab ab ab ab ab ab 00 f0 ae 0c
^~~~~~~~~~~~~~~~~~~~~~~
vptr for 'electron::OffScreenRenderWidgetHostView'
#0 0x000300737f0c in electron::api::MenuMac::PopupOnUI(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x737f0c)
#1 0x000300739c70 in void base::internal::DecayedFunctorTraits<void (electron::api::MenuMac::*)(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>), electron::api::MenuMac*, base::WeakPtr<electron::NativeWindow>&&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>&&, int&&, int&&, int&&, int&&, base::OnceCallback<void ()>&&>::Invoke<void (electron::api::MenuMac::*)(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>), electron::api::MenuMac* const&, base::WeakPtr<electron::NativeWindow>, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>>(void (electron::api::MenuMac::*)(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>), electron::api::MenuMac* const&, base::WeakPtr<electron::NativeWindow>&&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>&&, int&&, int&&, int&&, int&&, base::OnceCallback<void ()>&&) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x739c70)
#2 0x000300739b08 in base::internal::Invoker<base::internal::FunctorTraits<void (electron::api::MenuMac::*&&)(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>), electron::api::MenuMac*, base::WeakPtr<electron::NativeWindow>&&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>&&, int&&, int&&, int&&, int&&, base::OnceCallback<void ()>&&>, base::internal::BindState<true, true, false, void (electron::api::MenuMac::*)(base::WeakPtr<electron::NativeWindow> const&, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>), cppgc::internal::BasicPersistent<gin::WeakCell<electron::api::MenuMac>, cppgc::internal::StrongPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, base::WeakPtr<electron::NativeWindow>, cppgc::internal::BasicPersistent<electron::api::WebFrameMain, cppgc::internal::WeakPersistentPolicy, cppgc::internal::KeepLocationPolicy, cppgc::internal::DisabledCheckingPolicy>, int, int, int, int, base::OnceCallback<void ()>>, void ()>::RunOnce(base::internal::BindStateBase*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x739b08)
#3 0x00030004d8e0 in base::OnceCallback<void ()>::Run() && (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x4d8e0)
#4 0x00030decedd4 in base::TaskAnnotator::RunTaskImpl(base::PendingTask&) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xdecedd4)
#5 0x00030df2e0d0 in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xdf2e0d0)
#6 0x00030df2d3fc in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xdf2d3fc)
#7 0x00030e03acfc in base::MessagePumpCFRunLoopBase::RunWork() (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xe03acfc)
#8 0x00030e03817c in base::apple::CallWithEHFrame(void () block_pointer) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xe03817c)
#9 0x00030e039750 in base::MessagePumpCFRunLoopBase::RunWorkSource(void*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xe039750)
#10 0x0001866eaa60 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:arm64e+0x7ca60)
#11 0x0001866ea9f4 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:arm64e+0x7c9f4)
#12 0x0001866ea760 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:arm64e+0x7c760)
#13 0x0001866e93b4 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:arm64e+0x7b3b4)
#14 0x0001866e89e4 (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation:arm64e+0x7a9e4)
#15 0x00019218e278 (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox:arm64e+0xc3278)
#16 0x0001921914e4 (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox:arm64e+0xc64e4)
#17 0x00019231c480 (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox:arm64e+0x251480)
#18 0x00018a609a30 (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit:arm64e+0x3aa30)
#19 0x00018afa85c8 (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit:arm64e+0x9d95c8)
#20 0x00018a5fcbe0 (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit:arm64e+0x2dbe0)
#21 0x00030e03c7ec in base::MessagePumpNSApplication::DoRun(base::MessagePump::Delegate*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xe03c7ec)
#22 0x00030e03831c in base::MessagePumpCFRunLoopBase::Run(base::MessagePump::Delegate*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xe03831c)
#23 0x00030df2f97c in base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool, base::TimeDelta) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xdf2f97c)
#24 0x00030de7fa18 in base::RunLoop::Run(base::Location const&) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0xde7fa18)
#25 0x000309c10454 in content::BrowserMainLoop::RunMainMessageLoop() (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x9c10454)
#26 0x000309c17780 in content::BrowserMainRunnerImpl::Run() (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x9c17780)
#27 0x000309c084d4 in content::BrowserMain(content::MainFunctionParams) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x9c084d4)
#28 0x0003013db724 in content::RunBrowserProcessMain(content::MainFunctionParams, content::ContentMainDelegate*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x13db724)
#29 0x0003013dfd8c in content::ContentMainRunnerImpl::RunBrowser(content::MainFunctionParams, bool) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x13dfd8c)
#30 0x0003013dee6c in content::ContentMainRunnerImpl::Run() (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x13dee6c)
#31 0x0003013da230 in content::RunContentProcess(content::ContentMainParams, content::ContentMainRunner*) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x13da230)
#32 0x0003013da548 in content::ContentMain(content::ContentMainParams) (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x13da548)
#33 0x000300028c1c in ElectronMain (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/Frameworks/Electron Framework.framework/Versions/A/Electron Framework:arm64+0x28c1c)
#34 0x00010406c684 in main (/Users/runner/work/electron/electron/src/out/Default/Electron.app/Contents/MacOS/Electron:arm64+0x100000684)
#35 0x00018625eb94 in start (/usr/lib/dyld:arm64e+0x6b94)
Checklist
- I have built and tested this change
- I have filled out the PR description
- I have reviewed and verified the changes
-
npm testpasses - tests are changed or added
- PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.
Release Notes
Notes: Fixed a potential crash when using menu.popup with a frame from an offscreen rendered window
Backports
42-x-y
PendingWaiting for a manual backport
43-x-y
PendingWaiting for a manual backport
44-x-y
PendingWaiting for a manual backport
Semver Impact
Major
Breaking changes
Minor
New features
Patch
Bug fixes
None
Docs, tests, etc.
Semantic Versioning helps users understand the impact of updates:
- Major (X.y.z): Breaking changes that may require code modifications
- Minor (x.Y.z): New features that maintain backward compatibility
- Patch (x.y.Z): Bug fixes that don't change the API
- None: Changes that don't affect using facing parts of Electron