[Charles Levert <chuck@xxxxxxxxxxxxxxxx>]

Hi.

When displaying strings inside "UDP/DCE RPC/Microsoft Messenger Service"
packets (such as that lovely spam promoting www.windowspatch.net), the
"actual character count" was taken at face value and not checked against
the total size of the tvb; an intentional overflow (or exception) could
have been provoked by a carefully crafted packet.

Also, the message string can be quite long and its display on a single
line ends up being truncated.  Since this is UDP, the "Follow TCP Stream"
feature cannot be used to view the message in ASCII lines.  Therefore,
it is useful to add a subtree that displays each line individually
(when there is more than one), instead of having to rely solely on the
"packet octets pane".  Keep in mind that the string content could have
been (maliciously?) crafted in an unusual way such that it contains
printable characters beyond the first NUL; therefore, it is a good idea
to check if lines can still be displayed past that point.

When the message is in wide (16-bit) characters, it is less confusing
and more consistent with the "Follow TCP Stream" display to substitute a
'.' for code points beyond the 8-bit range, rather than the character
that happens to have the same lower 8 bits (possibly NUL or another
control character).  Also, a check for endianness was missing.



[ Edited with ude,
    the unified-format diff-output ("unidiff") editor,
    version 0.1 of 2004-11-03.  ]
--- epan/tvbuff.c.svn-11796	2004-10-31 01:40:41 -0400
+++ epan/tvbuff.c	2004-11-11 20:34:05 -0500
@@ -12 +12 @@
  * $Id: tvbuff.c 11796 2004-08-22 00:31:58Z guy $
@@ -1645,7 +1645,7 @@ tvb_fake_unicode(tvbuff_t *tvb, int offs
 	for (i = 0; i < len; i++) {
 		character = little_endian ? tvb_get_letohs(tvb, offset)
 					  : tvb_get_ntohs(tvb, offset);
-		buffer[i] = character & 0xff;
+		buffer[i] = character < 256 ? character : '.';
 		offset += 2;
 	}
 
--- epan/dissectors/packet-dcerpc.c.svn-12128	2004-10-30 00:37:33 -0400
+++ epan/dissectors/packet-dcerpc.c	2004-11-12 01:43:46 -0500
@@ -6 +6 @@
  * $Id: packet-dcerpc.c 12128 2004-09-29 00:06:36Z guy $
@@ -33,6 +33,7 @@
 
 #include <glib.h>
 #include <epan/packet.h>
+#include <epan/strutil.h>
 #include <epan/dissectors/packet-dcerpc.h>
 #include <epan/conversation.h>
 #include <epan/prefs.h>
@@ -413,6 +414,7 @@
 static gint ett_dcerpc_dg_flags2 = -1;
 static gint ett_dcerpc_pointer_data = -1;
 static gint ett_dcerpc_string = -1;
+static gint ett_dcerpc_line = -1;
 static gint ett_dcerpc_fragments = -1;
 static gint ett_dcerpc_fragment = -1;
 static gint ett_dcerpc_krb5_auth_verf = -1;
@@ -1242,7 +1244,7 @@ dissect_ndr_cvstring(tvbuff_t *tvb, int 
     proto_item *string_item;
     proto_tree *string_tree;
     guint32 len, buffer_len;
-    char *s;
+    char *s, *s1;
     header_field_info *hfinfo;
 
     di=pinfo->private_data;
@@ -1251,7 +1253,7 @@ dissect_ndr_cvstring(tvbuff_t *tvb, int 
       return offset;
     }
 
-    if (add_subtree) {
+    if (tree && add_subtree) {
         string_item = proto_tree_add_text(tree, tvb, offset, -1, "%s",
                                           proto_registrar_get_name(hfindex));
         string_tree = proto_item_add_subtree(string_item, ett_dcerpc_string);
@@ -1277,9 +1279,15 @@ dissect_ndr_cvstring(tvbuff_t *tvb, int 
     if (offset % size_is)
         offset += size_is - (offset % size_is);
 
+    len = tvb_length_remaining(tvb, offset);
+    if (len < buffer_len)
+	buffer_len = len;
+
+    s1 = NULL;
+
     if (size_is == sizeof(guint16)) {
         /* XXX - use drep to determine the byte order? */
-        s = tvb_fake_unicode(tvb, offset, buffer_len / 2, TRUE);
+        s = tvb_fake_unicode(tvb, offset, buffer_len / 2, drep[0] & 0x10);
         /*
          * XXX - we don't support a string type with Unicode
          * characters, so if this is a string item, we make
@@ -1291,8 +1299,7 @@ dissect_ndr_cvstring(tvbuff_t *tvb, int 
                 proto_tree_add_string(string_tree, hfindex, tvb, offset,
                                       buffer_len, s);
             } else {
-                proto_tree_add_item(string_tree, hfindex, tvb, offset,
-                                    buffer_len, drep[0] & 0x10);
+                s1 = s;
             }
         }
     } else {
@@ -1306,10 +1313,48 @@ dissect_ndr_cvstring(tvbuff_t *tvb, int 
          */
         s = tvb_get_string(tvb, offset, buffer_len);
         if (tree && buffer_len)
-            proto_tree_add_item(string_tree, hfindex, tvb, offset,
-                                buffer_len, drep[0] & 0x10);
+            s1 = s;
     }
 
+    if (s1) {
+	proto_item *line_item;
+	char *s2;
+	char *end = s + buffer_len / size_is;
+
+	line_item = proto_tree_add_item(string_tree, hfindex, tvb, offset,
+					buffer_len, drep[0] & 0x10);
+	s2 = strchr(s1, '\n');
+	if (s2)
+	    s2++;
+	else
+	    s2 = s1 + strlen(s1);
+	if (s2 < end) {
+	    proto_tree *line_tree;
+
+	    line_tree = proto_item_add_subtree(line_item, ett_dcerpc_line);
+	    for (;;) {
+		len = s2 - s1;
+		if (!len) {
+			while (s2 < end && !*s2)
+				s2++;
+			len = s2 - s1;
+		}
+		proto_tree_add_text(line_tree, tvb,
+				    offset + (s1 - s) * size_is,
+				    len * size_is,
+				    "%s", format_text(s1, len));
+		if (s2 >= end)
+		    break;
+		s1 = s2;
+		s2 = strchr(s1, '\n');
+		if (s2)
+		    s2++;
+		else
+		    s2 = s1 + strlen(s1);
+	    }
+	}
+    }
+
     if (string_item != NULL)
         proto_item_append_text(string_item, ": %s", s);
 
@@ -4835,6 +4880,7 @@ proto_register_dcerpc (void)
         &ett_dcerpc_dg_flags2,
         &ett_dcerpc_pointer_data,
         &ett_dcerpc_string,
+        &ett_dcerpc_line,
         &ett_dcerpc_fragments,
         &ett_dcerpc_fragment,
         &ett_dcerpc_krb5_auth_verf,