[Gilbert Ramirez <gram@xxxxxxxxxx>]

On Fri, Feb 16, 2001 at 11:40:23AM +0100, Dick Gooris wrote:
> Guy,
> 
> I think I have found an issue which may need some attention. I am
> building
> a protocol Sapphire (A protocol to analyze control traffic between
> Signalling
> gateways, and atm access concentrators) 
> 
> I start ethereal with :
> 
> 	./ethereal -r debugdata
> 
> I can the see all the sapphire messages, and look quite well.
> Then once it is processed, I want to get rid of overhead, so I type at
> the bottom : 
> tcp.port == 7777  A small window is opened, with 'Filtering', and
> halfway it hangs forever.  

There were 2 bugs in the diameter dissector having to do with not
checking the value of a field taken from the packet data.

The first caused a segfault on my machine. The second caused
the infinitte loop that you reported.

Attached is a diff against Ethereal 0.8.15.

thanks!

--gilbert

Index: packet-diameter.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-diameter.c,v
retrieving revision 1.11
retrieving revision 1.14
diff -u -r1.11 -r1.14
--- packet-diameter.c	2001/01/09 06:31:35	1.11
+++ packet-diameter.c	2001/02/16 21:44:54	1.14
@@ -1,7 +1,7 @@
 /* packet-diameter.c
  * Routines for DIAMETER packet disassembly
  *
- * $Id: packet-diameter.c,v 1.11 2001/01/09 06:31:35 guy Exp $
+ * $Id: packet-diameter.c,v 1.14 2001/02/16 21:44:54 gram Exp $
  *
  * Copyright (c) 2000 by David Frascone <chaos@xxxxxxxxxxxxxx>
  *
@@ -150,7 +150,7 @@
   return(0);
 }
 
-static gchar *rdconvertbufftostr(gchar *dest,guint8 length,const guint8 *pd)
+static gchar *rdconvertbufftostr(gchar *dest,int length,const guint8 *pd)
 {
 /*converts the raw buffer into printable text */
 guint32 i;
@@ -306,7 +306,7 @@
 	guint32 intval;
 	int dataLen;
 	char *valstr;
-	static char buffer[1024];
+	static char buffer[1024 + 7]; /* 7 = strlen("Value: ") */
 
 	dataLen = avph->avp_length - sizeof(e_avphdr);
 
@@ -315,6 +315,13 @@
 	if (!(avph->avp_flags & AVP_FLAGS_T))
 		dataLen += 4;
 
+	if (dataLen < 0) {
+		return "Data Length too small.";
+	}
+	else if (dataLen >= sizeof(buffer)) {
+		return "Data Length too big.";
+	}
+
 /* prints the values of the attribute value pairs into a text buffer */
 	
 	print_type=match_numval(avph->avp_type,diameter_printinfo);
@@ -387,17 +394,23 @@
 	guint32 vendorId=0;
 	int dataOffset;
 	int fixAmt;
+	int adj;
 	proto_item *avptf;
 	proto_tree *avptree;
 	int vendorOffset, tagOffset;
 	
-	if (avplength==0) {
+	if (avplength <= 0) {
 		proto_tree_add_text(tree, NullTVB,offset,0,
 		    "No Attribute Value Pairs Found");
 		return;
 	}
-	
+
 	while (avplength > 0 ) {
+
+		if (! IS_DATA_IN_FRAME(offset)) {
+			break;
+		}
+
 		vendorOffset = tagOffset = 0;
 		memcpy(&avph,&pd[offset],sizeof(e_avphdr));
 		avph.avp_type = ntohl(avph.avp_type);
@@ -433,7 +446,8 @@
 		 */
 		fixAmt = 4 - (avph.avp_length % 4);
 		if (fixAmt == 4) fixAmt = 0;
-		avplength=avplength - (avph.avp_length + fixAmt);
+		adj = avph.avp_length + fixAmt;
+		avplength -= adj;
 		avptpstrval=match_strval(avph.avp_type, diameter_attrib_type_vals);
 		if (avptpstrval == NULL) avptpstrval="Unknown Type";
 		if (!BYTES_ARE_IN_FRAME(offset, avph.avp_length)) {
@@ -480,8 +494,11 @@
 			    offset+dataOffset, avph.avp_length - dataOffset,
 			    "Data: (%d bytes) %s",
 			    avph.avp_length - dataOffset, valstr);
+		}
+		if (adj <= 0) {
+			break;
 		}
-		offset=offset+avph.avp_length + fixAmt;
+		offset += adj;
 	}
 }