Ethereal-dev: Re: [Ethereal-dev] Small oddity in ENIP in 0.9.15

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

From: Guy Harris <guy@xxxxxxxxxxxx>
Date: Mon, 6 Oct 2003 01:12:58 -0700
On Sat, Oct 04, 2003 at 03:14:03PM +0100, jfielding@xxxxxxxxxxxxxxx wrote:
> This bug happens in an unconnected send when MR size is odd, so there's a 
> pad byte after the Message Request. The dissector thinks the pad byte is 
> the last byte of the MR data, and is out by one byte till the end of the 
> message.

That code might have been easier to get right had the "offset" variable
been updated as the packet was dissected, rather than having the code
compute offsets for fields based on various variables.

I've attached a patch, which I've checked in, that fixes this.

> Expand frame 10 until you can see "Message Request Size: 0x0009".

Is networking a field that attracts people with 16 fingers?  (I.e., why
do several dissectors display sizes, counts, etc. in hex rather than
decimal?)
Index: packet-enip.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-enip.c,v
retrieving revision 1.6
diff -c -r1.6 packet-enip.c
*** packet-enip.c	1 Oct 2003 21:51:59 -0000	1.6
--- packet-enip.c	6 Oct 2003 08:09:59 -0000
***************
*** 1673,1689 ****
              if( msg_req_siz % 2 )
              {
                 /* Pad byte */
!                proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+6+msg_req_siz-3, 1, "Pad Byte (0x%02X)",
!                   tvb_get_guint8( tvb, offset+2+req_path_size+6+msg_req_siz-3 ) );
              }
  
              /* Route Path Size */
!    		   route_path_size = tvb_get_guint8( tvb, offset+2+req_path_size+6+msg_req_siz-2 )*2;
!    		   proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+6+msg_req_siz-2, 1, "Route Path Size: %d (words)", route_path_size/2 );
  
!    		   /* Reserved */
!             proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+6+msg_req_siz-1, 1, "Reserved (0x%02X)",
!                 tvb_get_guint8( tvb, offset+2+req_path_size+6+msg_req_siz-1 ) );
  
              /* Route Path */
              temp_item = proto_tree_add_text(cmd_data_tree, tvb, offset+2+req_path_size+6+msg_req_siz, route_path_size, "Route Path");
--- 1673,1690 ----
              if( msg_req_siz % 2 )
              {
                 /* Pad byte */
!                proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+4+msg_req_siz, 1, "Pad Byte (0x%02X)",
!                   tvb_get_guint8( tvb, offset+2+req_path_size+4+msg_req_siz ) );
!                msg_req_siz++;	/* include the padding */
              }
  
              /* Route Path Size */
!             route_path_size = tvb_get_guint8( tvb, offset+2+req_path_size+4+msg_req_siz )*2;
!             proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+4+msg_req_siz, 1, "Route Path Size: %d (words)", route_path_size/2 );
  
!             /* Reserved */
!             proto_tree_add_text( cmd_data_tree, tvb, offset+2+req_path_size+5+msg_req_siz, 1, "Reserved (0x%02X)",
!                 tvb_get_guint8( tvb, offset+2+req_path_size+5+msg_req_siz ) );
  
              /* Route Path */
              temp_item = proto_tree_add_text(cmd_data_tree, tvb, offset+2+req_path_size+6+msg_req_siz, route_path_size, "Route Path");