talons

commit a73b1eec22aeb59262113ba7044081349da446db
parent 0ae08c27c278560b157259b62b09fb8244e96e53
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
@@ -1784,7 +1784,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);