Ethereal-dev: Re: [Ethereal-dev] Error in TCP/TPKT desegmenting

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Guy Harris <guy@xxxxxxxxxx>
Date: Wed, 31 Jul 2002 11:43:50 -0700
On Wed, Jul 31, 2002 at 01:49:15PM +0200, Miodrag Marinkovic wrote:
> By the way, the described problems occuried with 
> Ethereal 0.9.4 and 0.9.5 on Windows NT 4.0. 

...and with Ethereal 0.9.5, and current CVS Ethereal, on Solaris 8.  (It
doesn't matter whether there's a display filter or not; the display
doesn't change.)

The problem is that it's not checking the version field.  Given that
your capture appears to start in the *middle* of a TPKT PDU, the very
first TPKT TCP segment packet contains data that is *not* a TPKT header;
however, the dissector still extracts the length and tries to use it to
do desegmentation.

That problem can't be fixed 100%, as you might be unlucky enough to have
the first TPKT TCP segment in a capture begin with a byte with the value
3.

However, if we check for a version number of 3, and, if the version
number *isn't* 3, just treat the rest of the TCP segment as
"continuation data", that appears to fix the problem.

I've attached the patch to "packet-tpkt.c" that I'll be checking in.
Index: packet-tpkt.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-tpkt.c,v
retrieving revision 1.19
diff -c -r1.19 packet-tpkt.c
*** packet-tpkt.c	13 May 2002 21:18:25 -0000	1.19
--- packet-tpkt.c	31 Jul 2002 18:39:41 -0000
***************
*** 144,149 ****
--- 144,175 ----
  		col_add_str(pinfo->cinfo, COL_INFO, "");
    
  	while (tvb_reported_length_remaining(tvb, offset) != 0) {
+ 		/*
+ 		 * Is the first byte of this putative TPKT header
+ 		 * a valid TPKT version number, i.e. 3?
+ 		 */
+ 		if (tvb_get_guint8(tvb, offset) != 3) {
+ 			/*
+ 			 * No, so don't assume this is a TPKT header;
+ 			 * we might be in the middle of TPKT data,
+ 			 * so don't get the length and don't try to
+ 			 * do reassembly.
+ 			 */
+ 			if (check_col(pinfo->cinfo, COL_PROTOCOL))
+ 				col_set_str(pinfo->cinfo, COL_PROTOCOL, "TPKT");
+ 			if (check_col(pinfo->cinfo, COL_INFO))
+ 				col_set_str(pinfo->cinfo, COL_INFO, "Continuation");
+ 			if (tree) {
+ 				ti = proto_tree_add_item(tree, proto_tpkt, tvb,
+ 				    offset, -1, FALSE);
+ 				tpkt_tree = proto_item_add_subtree(ti, ett_tpkt);
+ 
+ 				proto_tree_add_text(tpkt_tree, tvb, offset, -1,
+ 				    "Continuation data");
+ 			}
+ 			return;
+ 		}
+ 
  		length_remaining = tvb_length_remaining(tvb, offset);
  
  		/*