[Guy Harris <gharris@xxxxxxxxx>]

On Sat, Jul 26, 2003 at 01:34:32AM -0700, Guy Harris wrote:
> I don't have time right now to rewrite it (and don't know when I will,
> if ever),

I did, however, find time to put in some crude checks to keep it from
crashing on *some* MEGACO packets, including the one you sent; I've
checked in a fix, and attached a patch.

The parser probably should be significantly changed, however, to
properly lexically analyze packets.

Index: plugins/megaco/packet-megaco.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/plugins/megaco/packet-megaco.c,v
retrieving revision 1.8
diff -c -r1.8 plugins/megaco/packet-megaco.c
*** plugins/megaco/packet-megaco.c	26 Jul 2003 04:51:08 -0000	1.8
--- plugins/megaco/packet-megaco.c	30 Jul 2003 06:28:31 -0000
***************
*** 282,287 ****
--- 282,298 ----
  	if (tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E') != -1 && tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E') < tvb_current_offset) 
  		tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, 'E');
  	
+ 	if (tvb_current_offset == -1) {
+ 		ti = proto_tree_add_item(tree,proto_megaco,tvb, 0, -1, FALSE);
+ 		megaco_tree = proto_item_add_subtree(ti, ett_megaco);
+ 		proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ 		    "Sorry, no \"=\" in this packet, I can't parse it");
+ 		return;
+ 	}
+ 	/*
+ 	 * "tvb_previous_offset" will only be set if the corresponding
+ 	 * "tvb_find_guint8()" didn't return -1, so it's not -1.
+ 	 */
  	len = tvb_current_offset - tvb_previous_offset;
  	
  	tvb_get_nstringz0(tvb,tvb_previous_offset,len+1,transaction);	
***************
*** 290,295 ****
--- 301,313 ----
  	
  	tvb_current_offset  = tvb_find_guint8(tvb, tvb_offset,
  		tvb_len, '{');
+ 	if (tvb_current_offset == -1) {
+ 		ti = proto_tree_add_item(tree,proto_megaco,tvb, 0, -1, FALSE);
+ 		megaco_tree = proto_item_add_subtree(ti, ett_megaco);
+ 		proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ 		    "Sorry, no \"{\" in this packet, I can't parse it");
+ 		return;
+ 	}
  	
  	len = tvb_current_offset - tvb_offset;
  	
***************
*** 333,341 ****
--- 351,369 ----
  		/* Find version */
  		tvb_previous_offset = tvb_find_guint8(tvb, 0,
  			tvb_len, '/') + 1;
+ 		if (tvb_previous_offset == -1) {
+ 			proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ 			    "Sorry, no \"/\" in the MEGACO header, I can't parse this packet");
+ 			return;
+ 		}
  		
  		tvb_current_offset  = tvb_find_guint8(tvb, tvb_previous_offset,
  			tvb_len, ' ');
+ 		if (tvb_previous_offset == -1) {
+ 			proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ 			    "Sorry, no \" \" after the \"/\" in the MEGACO header, I can't parse this packet");
+ 			return;
+ 		}
  		
  		tokenlen = tvb_current_offset - tvb_previous_offset;
  		
***************
*** 348,362 ****
  		/* Find transaction */
  		tvb_offset = tvb_find_guint8(tvb, 0,
  			tvb_len, ':');
- 		tvb_current_offset = tvb_find_guint8(tvb, 0,
- 			tvb_len, '=');
  		
  		/* Transaction / TransactionResponseAck */
  		
  		tvb_current_offset = tvb_find_guint8(tvb, 0,
  			tvb_len, '=');
  		
  		tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, transaction[0]);
  		
  		tokenlen = tvb_current_offset - tvb_previous_offset;
  		
--- 376,395 ----
  		/* Find transaction */
  		tvb_offset = tvb_find_guint8(tvb, 0,
  			tvb_len, ':');
  		
  		/* Transaction / TransactionResponseAck */
  		
+ 		/* We did this earlier, so we know it doesn't fail */
  		tvb_current_offset = tvb_find_guint8(tvb, 0,
  			tvb_len, '=');
  		
  		tvb_previous_offset = tvb_find_guint8(tvb, tvb_offset, tvb_len, transaction[0]);
+ 		if (tvb_previous_offset == -1) {
+ 			proto_tree_add_text(megaco_tree, tvb, 0, -1,
+ 			    "Sorry, no \"%c\" past the \":\" in this packet, I can't parse it",
+ 			    transaction[0]);
+ 			return;
+ 		}
  		
  		tokenlen = tvb_current_offset - tvb_previous_offset;