-
Notifications
You must be signed in to change notification settings - Fork 50
Description
Summary
Calling set_menu() on a tray icon (via tray-icon) while the menu is currently displayed causes a use-after-free. If the user then clicks a menu item, the app crashes inside fire_menu_item_click with a panic in to_png() (ZeroWidth) or a segfault — depending on what the freed memory looks like.
Root cause
MudaMenuItem stores a raw *const MenuChild pointer as an ivar (mod.rs:947):
// FIXME: Use `Rc` or something else to access the MenuChild.
#[ivars = Cell<*const MenuChild>]
struct MenuItem;When set_menu(new_menu) is called:
- The old
Box<dyn ContextMenu>is dropped →MenuChilditems are freed - But old
NSMenuItems survive (retained by the still-visibleNSMenu) - User clicks a stale menu item →
fire_menu_item_clickdereferences the freed pointer - Use-after-free → reads garbage → crash
Crash we hit
PANIC on thread 'main' at muda-0.17.1/src/platform_impl/macos/icon.rs:34:53:
called `Result::unwrap()` on an `Err` value: Format(FormatError { inner: ZeroWidth })
The freed memory was reinterpreted as a MenuChild with an Icon containing width=0. to_png() encodes it → png crate rejects zero width → .unwrap() panics. Since this runs inside an extern "C" Obj-C callback (fireMenuItemAction:), the panic crosses a nounwind boundary → immediate abort. catch_unwind cannot help.
Secondary issue: to_png() uses .unwrap()
Even without the use-after-free, PlatformIcon::to_png() (icon.rs:34) should return Result instead of using .unwrap(). The tray-icon crate's version of the same function already does this. And Icon::from_rgba(vec![], 0, 0) currently succeeds (the validation allows zero dimensions), so a zero-dimension Icon can be constructed legally.
Reproduction
This triggers in any app that periodically calls tray.set_menu(Some(new_menu)) to update the tray menu while the user has the menu open. Related to #128, #202, #233 — all are manifestations of the dangling *const MenuChild.
Workaround
Keep the previous Menu alive (e.g. via menu.clone() into a static) so the Rc<RefCell<MenuChild>> ref count stays > 0 until the next update cycle.
Suggested fix
Replace Cell<*const MenuChild> with Rc<MenuChild> (as the FIXME already suggests). This would make the MenuChild lifetime tied to the NSMenuItem, preventing the use-after-free entirely.