Zander671/Waybar
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(tray): stop reorderItems from re-adding items (iterator UAF + double add)

Browse Source 
The item-ordering feature made Host::reorderItems() re-run the full
remove/add path over items_ via std::ranges::for_each(on_remove_/on_add_).
This caused two confirmed bugs:

BUG 1 (iterator invalidation / UAF): on_add_ (Tray::onAdd) calls
Host::checkIgnoreList, which erases from items_ while for_each is still
iterating items_, invalidating iterators/pointers. Triggered by a
non-empty ignore-list matching an item with >=2 items present.

BUG 2 (double add): reorderItems runs while an item's Id is resolved in
proxyReady, i.e. before setReady(). It added the not-yet-ready item
(re-parenting its event_box, pushing into Tray::items_, connecting
signal_show/hide), then setReady() -> itemReady -> onAdd added it again:
GTK 'widget already has a parent' critical, duplicate Item* and
signal handlers that accumulated unbounded.

Fixes:
- reorderItems() now only reorders already-added GTK box children via a
  dedicated on_reorder_ callback (Tray::reorderBox), never re-adding or
  removing. reorderBox stable-sorts items_ by order_ and repositions
  children with gtk_box_reorder_child (honouring reverse-direction).
- Tray::onAdd is idempotent (guards against an already-added item) and
  positions the new widget via reorderBox before the ignore-list check.
- signal_show/signal_hide connections are stored per item and
  disconnected in Tray::onRemove; onRemove is a no-op for items that were
  never added.
This commit is contained in:
Alex
2026-07-05 10:13:24 +02:00
parent 74cf45d530
commit 66139e4440
4 changed files with 72 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/modules/sni/host.cpp
+11 -6
View File
@@ -16,7 +16,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar,
const std::vector<std::string>& ignore_list,
const std::function<void(std::unique_ptr<Item>&)>& on_add,
const std::function<void(std::unique_ptr<Item>&)>& on_remove,
const std::function<void()>& on_update)
const std::function<void()>& on_reorder, const std::function<void()>& on_update)
: bus_name_("org.kde.StatusNotifierHost-" + std::to_string(getpid()) + "-" +
std::to_string(id)),
object_path_("/StatusNotifierHost/" + std::to_string(id)),
@@ -27,6 +27,7 @@ Host::Host(std::size_t id, const Json::Value& config, const Bar& bar,
ignore_list_(ignore_list),
on_add_(on_add),
on_remove_(on_remove),
on_reorder_(on_reorder),
on_update_(on_update) {
auto orders = config["orders"];
if (!orders.isNull()) {
@@ -292,11 +293,15 @@ void Host::addRegisteredItem(const std::string& service) {
}
void Host::reorderItems() {
std::ranges::for_each(items_, on_remove_);
std::ranges::sort(items_, [](std::unique_ptr<Item>& item1, std::unique_ptr<Item>& item2) {
return item1->order_ < item2->order_;
});
std::ranges::for_each(items_, on_add_);
// Re-apply the configured ordering to the tray. This is invoked while an
// item's Id/order is first resolved (from Item::setCustomIcon), which happens
// *before* the item is marked ready and added. It must therefore only reorder
// the widgets that have already been added; re-running the full add path here
// would (a) re-parent widgets and reconnect signals for every item and (b)
// mutate items_ from within checkIgnoreList while it is being iterated,
// invalidating iterators/pointers. Delegating to on_reorder_ keeps this to a
// pure reordering of existing children.
on_reorder_();
}
} // namespace waybar::modules::SNI