talons

commit 6023fa27735149267d4951b318720227deb231cb
parent 9cdf381a9a7bfefde0957531a2972b3e05ba0947
Author: Andrej Kacian <ticho@claws-mail.org>
Date:   Thu, 25 Apr 2019 00:13:35 +0200

Fix a long-standing use-after-free in mainwin_actions_execute()

message_actions_execute() eventually calls summary_show()
to redisplay current folder in summaryview.
This causes a summary_clear(), which frees all MsgInfos
from the local linked list in mainwin_actions_execute().
This list is then used to restore summaryview selection, but
at this point, all its members point to already freed memory.

We solve this by increasing each MsgInfo's reference count,
so that they do not get freed, and we free them after we're
done with them.

Note: procmsg_msginfo_free() should probably be renamed to
procmsg_msginfo_unref()

Diffstat:
M
src/action.c
 | 
8
++++++++
M
src/summaryview.c
 | 
3
++-

2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/action.c b/src/action.c
@@ -620,6 +620,13 @@ static void mainwin_actions_execute_cb(GtkWidget *widget, gpointer data)
 	mainwin_actions_execute(mainwin, action_nb, NULL);
 }
 
+static void _free_msginfos(gpointer data, gpointer user_data)
+{
+	MsgInfo *msginfo = (MsgInfo *)data;
+
+	procmsg_msginfo_free(&msginfo);
+}
+
 static void mainwin_actions_execute(MainWindow *mainwin, guint action_nb,
 				       GtkWidget *widget)
 {
@@ -628,6 +635,7 @@ static void mainwin_actions_execute(MainWindow *mainwin, guint action_nb,
 	msg_list = summary_get_selected_msg_list(mainwin->summaryview);
 	message_actions_execute(mainwin->messageview, action_nb, msg_list);
 	summary_select_by_msg_list(mainwin->summaryview, msg_list);
+	g_slist_foreach(msg_list, _free_msginfos, NULL);
 	g_slist_free(msg_list);
 }
 
diff --git a/src/summaryview.c b/src/summaryview.c
@@ -1798,7 +1798,8 @@ GSList *summary_get_selected_msg_list(SummaryView *summaryview)
 	for (cur = GTK_CMCLIST(summaryview->ctree)->selection; cur != NULL && cur->data != NULL;
 	     cur = cur->next) {
 		msginfo = GTKUT_CTREE_NODE_GET_ROW_DATA(cur->data);
-		mlist = g_slist_prepend(mlist, msginfo);
+		mlist = g_slist_prepend(mlist,
+				procmsg_msginfo_new_ref(msginfo));
 	}
 
 	mlist = g_slist_reverse(mlist);