Wireshark-dev: [Wireshark-dev] DND crash through all versions?

From: Jaap Keuter <jaap.keuter@xxxxxxxxx>
Date: Mon, 03 Sep 2012 23:50:25 +0200
Hi list,

This strikes me as too odd: on whatever version I tried (1.2, 1.4, 1.6. 1.8, head) when I DND a single capture file, it crashes.

Ok, bit of context: Compiled (32-bit) with GTK+ 2.24.10, with Cairo 1.12.2, with Pango 1.30.0, with GLib 2.32.3, etc.
Debian testing/KDE, but also FC14/KDE (not sure on the GTK/GDK there)

A little digging showed me that the URL(list) dnd_data_received() gets aren't /r/n terminated. If there are multiple files dropped all but the last are /r/n terminated, so if you drop two files it read the first, but not the last. When you drop one file it's seeing no file and goes on to assign to a zero sized array.

I've made a patch here that needs to be verified on other platforms first to see nothing breaks and further. Hope you can give it a go.

Anyone knows what strstr() does with a NULL for a haystack? As far as I can tell it's unspecified.

Thanks,
Jaap
--- ui/gtk/drag_and_drop.c.orig	2012-09-03 23:37:55.000000000 +0200
+++ ui/gtk/drag_and_drop.c	2012-09-03 23:35:52.000000000 +0200
@@ -228,7 +228,7 @@ dnd_open_file_cmd(gchar *cf_names_freeme
         in_files++;
     }
 
-    in_filenames = g_malloc(sizeof(char*) * in_files);
+    in_filenames = g_malloc(sizeof(char*) * (in_files + 1));
 
     /* store the starts of the file entries in a gchar array */
     cf_name = cf_names_freeme;
@@ -251,6 +251,7 @@ dnd_open_file_cmd(gchar *cf_names_freeme
     switch(in_files) {
     case(0):
         /* shouldn't happen */
+        g_error("Drag and drop to open file, but no filenames? (%s)", cf_name ? cf_name : "null");
         break;
     case(1):
         /* open and read the capture file (this will close an existing file) */
@@ -328,9 +329,15 @@ dnd_data_received(GtkWidget *widget _U_,
 	/* the data string is not zero terminated -> make a zero terminated "copy" of it */
 	sel_data_len = gtk_selection_data_get_length(selection_data);
 	sel_data_data = gtk_selection_data_get_data(selection_data);
-	cf_names_freeme = g_malloc(sel_data_len + 1);
+	cf_names_freeme = g_malloc(sel_data_len + 3);
 	memcpy(cf_names_freeme, sel_data_data, sel_data_len);
-	cf_names_freeme[sel_data_len] = '\0';
+	if (cf_names_freeme[sel_data_len - 1] == '\n')
+	    cf_names_freeme[sel_data_len] = '\0';
+	else {
+	    cf_names_freeme[sel_data_len] = '\r';
+	    cf_names_freeme[sel_data_len + 1] = '\n';
+	    cf_names_freeme[sel_data_len + 2] = '\0';
+	}
 
         /* If there's unsaved data, let the user save it first.
            If they cancel out of it, don't open the file. */