Wireshark · Ethereal-dev: Re: [Ethereal-dev] more mgcp fixes

[Ed Warnicke <hagbard@xxxxxxxxxxxxxxxxxxx>]

Did someone make these corrections already?  I didn't change these 
during my most recent attack on the MGCP plugin.  It looks like 
some kind soul brought me into a reasonable version of compliance in the 
current CVS verson.  I certainly don't object to this, I'll update 
before I hack on the file again.  

Here is a patch with the prefs fixed to what they had been.  
Please check it in.

Ed

P.S. Has anyone looked at the filter work I did?

On Wed, 27 Dec 2000, Guy Harris wrote:

> On Thu, Dec 28, 2000 at 12:59:28AM -0500, Ed Warnicke wrote:
> > Please check in these other mgcp plugin fixes.
> 
> 	...
> 
> > -  prefs_register_bool_preference(mgcp_module, "display_raw_text", 
> > +  prefs_register_bool_preference(mgcp_module, "display raw text toggle", 
> 
> Preference names aren't supposed to have blanks in them.  That makes it
> a pain to set them from the command line.  ("toggle" is also
> inconsistent with other preferences, including the option to control
> whether the tree dissection is supposed to be displayed.)
> 
> Yes, that means that there will be warnings until the preferences file
> is rewritten; it's a one-time pain.
> 

Index: plugins/mgcp/ChangeLog
===================================================================
RCS file: /cvsroot/ethereal/plugins/mgcp/ChangeLog,v
retrieving revision 1.1
diff -u -r1.1 ChangeLog
--- ChangeLog	2000/11/09 10:04:48	1.1
+++ ChangeLog	2000/12/28 06:35:11
@@ -0,0 +1,3 @@
+Overview of changes in Ethereal MGCP plugin 0.0.2:
+* Fixed misc problems with improperly dissecting some xgcp packets as 
+  short or malformed.  
Index: plugins/mgcp/moduleinfo.h
===================================================================
RCS file: /cvsroot/ethereal/plugins/mgcp/moduleinfo.h,v
retrieving revision 1.1
diff -u -r1.1 moduleinfo.h
--- moduleinfo.h	2000/11/09 10:04:48	1.1
+++ moduleinfo.h	2000/12/28 06:35:11
@@ -13,5 +13,5 @@
 #endif
 
 /* Version number of package */
-#define VERSION "0.0.1"
+#define VERSION "0.0.2"
 
Index: plugins/mgcp/packet-mgcp.c
===================================================================
RCS file: /cvsroot/ethereal/plugins/mgcp/packet-mgcp.c,v
retrieving revision 1.10
diff -u -r1.10 packet-mgcp.c
--- packet-mgcp.c	2000/12/25 09:37:35	1.10
+++ packet-mgcp.c	2000/12/28 06:35:11
@@ -2,7 +2,7 @@
  * Routines for mgcp packet disassembly
  * RFC 2705
  *
- * $Id: packet-mgcp.c,v 1.10 2000/12/25 09:37:35 guy Exp $
+ * $Id: packet-mgcp.c,v 1.9 2000/12/20 05:45:27 gram Exp $
  * 
  * Copyright (c) 2000 by Ed Warnicke <hagbard@xxxxxxxxxxxxxxxxxxx>
  *
@@ -812,15 +812,18 @@
       my_proto_tree_add_string = proto_tree_add_string_hidden;
     }
 
-    tvb_current_offset = tvb_find_guint8(tvb,tvb_previous_offset,
-					     tvb_current_len, ' ');
-    if(tvb_current_offset == -1){
-      tvb_current_offset = tvb_len;
-    }
-    tvb_current_len = tvb_length_remaining(tvb,tvb_previous_offset);
-    tokenlen = tvb_current_offset - tvb_previous_offset;
+    do {
+      tvb_current_len = tvb_length_remaining(tvb,tvb_previous_offset);
+      tvb_current_offset = tvb_find_guint8(tvb, tvb_previous_offset,
+					   tvb_current_len, ' ');
+      if(tvb_current_offset == -1){
+	tvb_current_offset = tvb_len;
+	tokenlen = tvb_current_len;
+      }				  
+      else{
+	tokenlen = tvb_current_offset - tvb_previous_offset;
+      }
 
-    while( tvb_current_offset < tvb_len || tokennum <= 3){
       if(tokennum == 0){
 	if(is_mgcp_verb(tvb,tvb_previous_offset,tvb_current_len)){
 	  mgcp_type = MGCP_REQUEST;
@@ -849,38 +852,46 @@
       if(tokennum == 2){
 	if(mgcp_type == MGCP_REQUEST){
 	  my_proto_tree_add_string(tree,hf_mgcp_req_endpoint, tvb,
-				tvb_previous_offset, tokenlen,
-				tvb_format_text(tvb, tvb_previous_offset,
-						tokenlen));
+				   tvb_previous_offset, tokenlen,
+				   tvb_format_text(tvb, tvb_previous_offset,
+						   tokenlen));
 	}
 	else if(mgcp_type == MGCP_RESPONSE){
-	  tokenlen = tvb_find_line_end(tvb, tvb_previous_offset, 
-				       -1,&tvb_current_offset);
-	  my_proto_tree_add_string(tree,hf_mgcp_rsp_rspstring, tvb,
+	  if(tvb_current_offset < tvb_len){
+	    tokenlen = tvb_find_line_end(tvb, tvb_previous_offset, 
+					 -1,&tvb_current_offset);
+	  }
+	  else{
+	    tokenlen = tvb_current_len;
+	  }
+	  my_proto_tree_add_string(tree, hf_mgcp_rsp_rspstring, tvb,
 				   tvb_previous_offset, tokenlen,
-				   tvb_format_text(tvb,tvb_previous_offset,
+				   tvb_format_text(tvb, tvb_previous_offset,
 						   tokenlen));
-	  break;
-	}
+	  }
+	break;
       }
       if( (tokennum == 3 && mgcp_type == MGCP_REQUEST) ){
-	tokenlen = tvb_find_line_end(tvb, tvb_previous_offset, 
-				     -1,&tvb_current_offset);
+	if(tvb_current_offset < tvb_len ){
+	  tokenlen = tvb_find_line_end(tvb, tvb_previous_offset, 
+				       -1,&tvb_current_offset);
+	}
+	else{
+	  tokenlen = tvb_current_len;
+	}
 	my_proto_tree_add_string(tree,hf_mgcp_version, tvb,
 				 tvb_previous_offset, tokenlen,
 				 tvb_format_text(tvb,tvb_previous_offset,
 						 tokenlen));
 	break;
       }
-      tvb_previous_offset = tvb_skip_wsp(tvb, tvb_current_offset,
-					 tvb_current_len);
-      tvb_current_len = tvb_length_remaining(tvb,tvb_previous_offset);
-      tvb_current_offset = tvb_find_guint8(tvb,tvb_previous_offset,
-					   tvb_current_len, ' ');
-      tokenlen = tvb_current_offset - tvb_previous_offset;
+      if(tvb_current_offset < tvb_len){
+	tvb_previous_offset = tvb_skip_wsp(tvb, tvb_current_offset,
+					   tvb_current_len);
+      }
       tokennum++;
-    }
-  
+    } while( tvb_current_offset < tvb_len && tvb_previous_offset < tvb_len 
+	     && tokennum <= 3);
     switch (mgcp_type){
     case MGCP_RESPONSE:
       proto_tree_add_boolean_hidden(tree,hf_mgcp_rsp, NullTVB,0,0,1);
@@ -972,8 +983,13 @@
  */
 static gint tvb_skip_wsp(tvbuff_t* tvb, gint offset, gint maxlength){
   gint counter = offset;
-  gint end = offset + maxlength;
+  gint end = offset + maxlength,tvb_len;
   guint8 tempchar;
+  tvb_len = tvb_length(tvb);
+  end = offset + maxlength;
+  if(end >= tvb_len){
+    end = tvb_len;
+  }
   for(counter = offset; counter < end && 
 	((tempchar = tvb_get_guint8(tvb,counter)) == ' ' || 
 	tempchar == '\t');counter++);