menu: fix unexpected behavior when a menu is opened from another menu

server->menu_current should be cleared before calling actions_run() as
it may internally call menu_open_root(). Clearing it after actions_run()
leads to an inconsistent state where a menu is opened but
server->menu_current is NULL. It even lead to a segfault when the item
opening another menu is contained in a pipemenu, because
menu_open_root() calls destroy_pipemenu() when server->menu_current is
set, which makes accessing item->actions a UAF.

diff --git a/src/menu/menu.c b/src/menu/menu.c
index a774842c624b891879f9a8749a178baa2ba7cee8..1ddfd7ebc0cc4aaaab4f6a7068e2df7306497f19 100644 (file)
--- a/src/menu/menu.c
+++ b/src/menu/menu.c
@@ -1720,13 +1720,9 @@ menu_execute_item(struct menuitem *item)
                return false;
        }
 
-       /*
-        * We close the menu here to provide a faster feedback to the user.
-        * We do that without resetting the input state so src/cursor.c
-        * can do its own clean up on the following RELEASE event.
-        */
        struct server *server = item->parent->server;
        menu_close(server->menu_current);
+       server->menu_current = NULL;
        seat_focus_override_end(&server->seat);
 
        /*
@@ -1746,7 +1742,6 @@ menu_execute_item(struct menuitem *item)
                                &item->actions, NULL);
        }
 
-       server->menu_current = NULL;
        destroy_pipemenus(server);
        return true;
 }