Fix crash in UDP Multicast Streams dialog

Attempting to open the UDP Multicast Streams dialog in the GTK UI
triggers an instant crash (heap-buffer-overflow).

Déjà vu. This is the same problem that plagued the RTP Streams dialog.
This patch is based on the fix in v1.99.3rc0-33-g2c65b33
(mcaststream_dlg_update confused GList vs. mcaststream_tapinfo_t).

After fixing that, the dialog crashed shortly after setting parameters
(heap-use-after-free). That fix is based on v1.99.10rc0-292-gb02a0ee
(after a retap, the old items were still present in the list).

Just that change was not enough as clearing the list still triggered a
signal, possibly because of the "changed" signal (while the RTP player
uses a selection setter function). Apply the patch based on
v1.99.10rc0-270-g01bd832 (disable selection while clearing).

Change-Id: I152bac6f954d8d1c5c20d6c7d56a196c3e20c681
Reviewed-on: https://code.wireshark.org/review/12227
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Alexis La Goutte <alexis.lagoutte@gmail.com>
(cherry picked from commit d7f12436709e40d58d7fcdfbcdd08740c039e162)
Reviewed-on: https://code.wireshark.org/review/12231

5 files changed, 42 insertions, 26 deletions

diff --git a/ui/gtk/mcast_stream_dlg.c b/ui/gtk/mcast_stream_dlg.c
index faba11c867..16ad6efd12 100644
--- a/ui/gtk/mcast_stream_dlg.c
+++ b/ui/gtk/mcast_stream_dlg.c
@@ -48,13 +48,14 @@
 #include "ui/gtk/gtkglobals.h"
 #include "ui/gtk/stock_icons.h"
 
-static void mcaststream_dlg_update(void *ti_ptr);
+static void mcaststream_tap_reset(mcaststream_tapinfo_t *tapinfo);
+static void mcaststream_tap_draw(mcaststream_tapinfo_t *tapinfo);
 
 /****************************************************************************/
 /* the one and only global mcaststream_tapinfo_t structure for tshark and wireshark.
  */
 static mcaststream_tapinfo_t the_tapinfo_struct =
-    {NULL, NULL, mcaststream_dlg_update, NULL, 0, NULL, FALSE};
+    {NULL, mcaststream_tap_reset, mcaststream_tap_draw, NULL, 0, NULL, FALSE};
 
 /* Capture callback data keys */
 #define E_MCAST_ENTRY_1     "burst_interval"
@@ -748,17 +749,8 @@ mcaststream_dlg_create(void)
 /* update the contents of the dialog box clist */
 /* list: pointer to list of mcast_stream_info_t* */
 void
-mcaststream_dlg_update(void *ti_ptr)
+mcaststream_dlg_update(GList *list)
 {
-    GList *list;
-    mcaststream_tapinfo_t *tapinfo = (mcaststream_tapinfo_t *)ti_ptr;
-
-    if (!tapinfo) {
-        return;
-    }
-
-    list = tapinfo->strinfo_list;
-
     if (mcast_stream_dlg != NULL) {
         gtk_list_store_clear(list_store);
         streams_nb = 0;
@@ -776,6 +768,29 @@ mcaststream_dlg_update(void *ti_ptr)
     last_list = list;
 }
 
+static void
+mcaststream_tap_reset(mcaststream_tapinfo_t *tapinfo _U_)
+{
+    GtkTreeSelection  *selection;
+    if (mcast_stream_dlg != NULL) {
+        /* Disable selection to avoid mcaststream_on_select_row from
+         * triggering and thereby accessing invalid memory. */
+        selection = gtk_tree_view_get_selection(GTK_TREE_VIEW(list_w));
+        gtk_tree_selection_set_mode(selection, GTK_SELECTION_NONE);
+        gtk_list_store_clear(list_store);
+        gtk_tree_selection_set_mode(selection, GTK_SELECTION_SINGLE);
+        streams_nb = 0;
+    }
+}
+
+static void
+mcaststream_tap_draw(mcaststream_tapinfo_t *tapinfo)
+{
+    if (tapinfo) {
+        mcaststream_dlg_update(tapinfo->strinfo_list);
+    }
+}
+
 #if 0
 static void
 mcaststream_dlg_mark_packet(mcaststream_tapinfo_t *tapinfo _U_, frame_data *fd) {
diff --git a/ui/mcast_stream.c b/ui/mcast_stream.c
index 24546c6584..79a5436fb8 100644
--- a/ui/mcast_stream.c
+++ b/ui/mcast_stream.c
@@ -121,7 +121,7 @@ mcaststream_reset_cb(void *ti_ptr)
     mcaststream_tapinfo_t *tapinfo = (mcaststream_tapinfo_t *)ti_ptr;
     if (tapinfo) {
         if (tapinfo->tap_reset) {
-           tapinfo->tap_reset(ti_ptr);
+           tapinfo->tap_reset(tapinfo);
         }
         mcaststream_reset(tapinfo);
     }
@@ -137,7 +137,7 @@ mcaststream_draw(void *ti_ptr)
     g_signal_emit_by_name(top_level, "signal_mcaststream_update");
 */
     if (tapinfo && tapinfo->tap_draw) {
-        tapinfo->tap_draw(ti_ptr);
+        tapinfo->tap_draw(tapinfo);
     }
     return;
 }
diff --git a/ui/mcast_stream.h b/ui/mcast_stream.h
index e780f8e885..bd871c485b 100644
--- a/ui/mcast_stream.h
+++ b/ui/mcast_stream.h
@@ -79,19 +79,23 @@ typedef struct _mcast_stream_info {
 
 } mcast_stream_info_t;
 
+typedef struct _mcaststream_tapinfo mcaststream_tapinfo_t;
+
+typedef void (*mcaststream_tap_reset_cb)(mcaststream_tapinfo_t *tapinfo);
+typedef void (*mcaststream_tap_draw_cb)(mcaststream_tapinfo_t *tapinfo);
 
 /* structure that holds the information about all detected streams */
 /* struct holding all information of the tap */
-typedef struct _mcaststream_tapinfo {
+struct _mcaststream_tapinfo {
     gpointer user_data;     /* User data pointer */
-    tap_reset_cb tap_reset; /**< tap reset callback */
-    tap_draw_cb tap_draw;   /**< tap draw callback */
+    mcaststream_tap_reset_cb tap_reset; /**< tap reset callback */
+    mcaststream_tap_draw_cb tap_draw;   /**< tap draw callback */
     GList*  strinfo_list;   /* list of mcast_stream_info_t */
     guint32 npackets;       /* total number of mcast packets of all streams */
     mcast_stream_info_t* allstreams; /* structure holding information common for all streams */
 
     gboolean is_registered; /* if the tap listener is currently registered or not */
-} mcaststream_tapinfo_t;
+};
 
 
 extern gint32  mcast_stream_trigger;
diff --git a/ui/qt/multicast_statistics_dialog.cpp b/ui/qt/multicast_statistics_dialog.cpp
index 02de8b1051..3246dc5871 100644
--- a/ui/qt/multicast_statistics_dialog.cpp
+++ b/ui/qt/multicast_statistics_dialog.cpp
@@ -21,8 +21,6 @@
 
 #include "multicast_statistics_dialog.h"
 
-#include "ui/mcast_stream.h"
-
 #include <QFormLayout>
 #include <QLabel>
 #include <QPushButton>
@@ -269,18 +267,16 @@ MulticastStatisticsDialog::~MulticastStatisticsDialog()
     delete tapinfo_;
 }
 
-void MulticastStatisticsDialog::tapReset(void *mti_ptr)
+void MulticastStatisticsDialog::tapReset(mcaststream_tapinfo_t *tapinfo)
 {
-    mcaststream_tapinfo_t *tapinfo = (mcaststream_tapinfo_t *)mti_ptr;
     MulticastStatisticsDialog *ms_dlg = dynamic_cast<MulticastStatisticsDialog *>((MulticastStatisticsDialog*)tapinfo->user_data);
     if (!ms_dlg || !ms_dlg->statsTreeWidget()) return;
 
     ms_dlg->statsTreeWidget()->clear();
 }
 
-void MulticastStatisticsDialog::tapDraw(void *mti_ptr)
+void MulticastStatisticsDialog::tapDraw(mcaststream_tapinfo_t *tapinfo)
 {
-    mcaststream_tapinfo_t *tapinfo = (mcaststream_tapinfo_t *)mti_ptr;
     MulticastStatisticsDialog *ms_dlg = dynamic_cast<MulticastStatisticsDialog *>((MulticastStatisticsDialog*)tapinfo->user_data);
     if (!ms_dlg || !ms_dlg->statsTreeWidget()) return;
 
diff --git a/ui/qt/multicast_statistics_dialog.h b/ui/qt/multicast_statistics_dialog.h
index 5e6a56d7f3..530761d163 100644
--- a/ui/qt/multicast_statistics_dialog.h
+++ b/ui/qt/multicast_statistics_dialog.h
@@ -23,6 +23,7 @@
 #define MULTICASTSTATISTICSDIALOG_H
 
 #include "tap_parameter_dialog.h"
+#include "ui/mcast_stream.h"
 
 class SyntaxLineEdit;
 
@@ -44,8 +45,8 @@ private:
     QList<QWidget *> line_edits_;
 
     // Callbacks for register_tap_listener
-    static void tapReset(void *mti_ptr);
-    static void tapDraw(void *mti_ptr);
+    static void tapReset(mcaststream_tapinfo_t *tapinfo);
+    static void tapDraw(mcaststream_tapinfo_t *tapinfo);
 
     virtual const QString filterExpression();