From d7cafa792f768d283398938b76c259baa2490d02 Mon Sep 17 00:00:00 2001 From: Marko Lindqvist Date: Sat, 7 Nov 2026 19:36:11 +0200 Subject: [PATCH 66/66] gtk4(x) gui_dialog_destroy(): Fix use-after-free Destroying the window or notebook page can lead to the destruction of the entire dialog structure. Make sure we don't access any members of it after that. This was leading to a crash on client launch on MacOS. Reported by Jan Szurek See RM #2447 Signed-off-by: Marko Lindqvist --- client/gui-gtk-4.0/gui_stuff.c | 24 +++++++++++++++--------- client/gui-gtk-5.0/gui_stuff.c | 24 +++++++++++++++--------- 2 files changed, 30 insertions(+), 18 deletions(-) diff --git a/client/gui-gtk-4.0/gui_stuff.c b/client/gui-gtk-4.0/gui_stuff.c index 5dd68c8b76..8e0d9972fa 100644 --- a/client/gui-gtk-4.0/gui_stuff.c +++ b/client/gui-gtk-4.0/gui_stuff.c @@ -873,31 +873,37 @@ void gui_dialog_set_title(struct gui_dialog *dlg, const char *title) **************************************************************************/ void gui_dialog_destroy(struct gui_dialog *dlg) { + if (dlg->source != NULL) { + *(dlg->source) = NULL; + } + switch (dlg->type) { case GUI_DIALOG_WINDOW: if (dlg->v.window != NULL) { + /* Destruction of the window can lead to the destruction of + * the entire dialog structure, via destrucion handler of + * dlg->v.grid. Be sure not to reference it after this. */ gtk_window_destroy(GTK_WINDOW(dlg->v.window)); - dlg->v.window = NULL; } break; case GUI_DIALOG_TAB: { gint n; + GtkNotebook *notebook = GTK_NOTEBOOK(dlg->v.tab.notebook); + gulong handler = dlg->v.tab.handler_id; - n = gtk_notebook_page_num(GTK_NOTEBOOK(dlg->v.tab.notebook), dlg->grid); + n = gtk_notebook_page_num(notebook, dlg->grid); fc_assert(n >= 0); - gtk_notebook_remove_page(GTK_NOTEBOOK(dlg->v.tab.notebook), n); - g_signal_handler_disconnect(GTK_NOTEBOOK(dlg->v.tab.notebook), - dlg->v.tab.handler_id); + /* Destruction of the notebook page can lead to the destruction of + * the entire dialog structure, via destrucion handler of + * dlg->v.grid. Be sure not to reference it after this. */ + gtk_notebook_remove_page(notebook, n); + g_signal_handler_disconnect(notebook, handler); } break; } - - if (dlg->source != NULL) { - *(dlg->source) = NULL; - } } /**********************************************************************//** diff --git a/client/gui-gtk-5.0/gui_stuff.c b/client/gui-gtk-5.0/gui_stuff.c index 656ba4c225..bd1aa715c1 100644 --- a/client/gui-gtk-5.0/gui_stuff.c +++ b/client/gui-gtk-5.0/gui_stuff.c @@ -874,31 +874,37 @@ void gui_dialog_set_title(struct gui_dialog *dlg, const char *title) **************************************************************************/ void gui_dialog_destroy(struct gui_dialog *dlg) { + if (dlg->source != nullptr) { + *(dlg->source) = nullptr; + } + switch (dlg->type) { case GUI_DIALOG_WINDOW: if (dlg->v.window != NULL) { + /* Destruction of the window can lead to the destruction of + * the entire dialog structure, via destrucion handler of + * dlg->v.grid. Be sure not to reference it after this. */ gtk_window_destroy(GTK_WINDOW(dlg->v.window)); - dlg->v.window = NULL; } break; case GUI_DIALOG_TAB: { gint n; + GtkNotebook *notebook = GTK_NOTEBOOK(dlg->v.tab.notebook); + gulong handler = dlg->v.tab.handler_id; - n = gtk_notebook_page_num(GTK_NOTEBOOK(dlg->v.tab.notebook), dlg->grid); + n = gtk_notebook_page_num(notebook, dlg->grid); fc_assert(n >= 0); - gtk_notebook_remove_page(GTK_NOTEBOOK(dlg->v.tab.notebook), n); - g_signal_handler_disconnect(GTK_NOTEBOOK(dlg->v.tab.notebook), - dlg->v.tab.handler_id); + /* Destruction of the notebook page can lead to the destruction of + * the entire dialog structure, via destrucion handler of + * dlg->v.grid. Be sure not to reference it after this. */ + gtk_notebook_remove_page(notebook, n); + g_signal_handler_disconnect(notebook, handler); } break; } - - if (dlg->source != nullptr) { - *(dlg->source) = nullptr; - } } /**********************************************************************//** -- 2.53.0