Ethereal-dev: [ethereal-dev] Padding patch for packet-smb.c

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

From: Mark Burton <markb@xxxxxxxxxx>
Date: Fri, 19 May 2000 20:32:45 +0100
Hi,

I have been using the marvellous ethereal to dissect some naughty SMB
packets and believe that ethereal's treatment of the pad1 & pad2
fields in some SMBs is incorrect. In particular, the original code
assumes that some data needs aligning on a 2 byte boundary but it may
be aligned on a 4 byte boundary (according to the CIFS/1.0 spec I
have).

The enclosed patch should improve matters.

I may well add some more dissecting code. If I do, I shall continue to
post patches to ethereal-dev unless otherwise advised.

Regards,

Mark

Index: packet-smb.c
===================================================================
RCS file: /cvsroot/ethereal/packet-smb.c,v
retrieving revision 1.65
diff -c -r1.65 packet-smb.c
*** packet-smb.c	2000/05/11 08:15:46	1.65
--- packet-smb.c	2000/05/19 18:30:16
***************
*** 8233,8240 ****
    guint8        SetupCount;
    guint8        Reserved3;
    guint8        Reserved1;
-   guint8        Pad2;
-   guint8        Pad1;
    guint8        MaxSetupCount;
    guint8        Data;
    guint32       Timeout;
--- 8233,8238 ----
***************
*** 8561,8580 ****
  
      }
  
!     if (offset % 2) {
  
!       /* Build display for: Pad1 */
  
!       Pad1 = GBYTE(pd, offset);
  
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, 1, "Pad1: %u", Pad1);
! 
        }
-     
-       offset += 1; /* Skip Pad1 */
  
      }
  
      if (ParameterCount > 0) {
--- 8559,8577 ----
  
      }
  
!     if (offset < (SMB_offset + ParameterOffset)) {
  
!       int pad1Count = SMB_offset + ParameterOffset - offset;
  
!       /* Build display for: Pad1 */
  
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, pad1Count, "Pad1: %s", format_text(pd + offset, pad1Count));
        }
  
+       offset += pad1Count; /* Skip Pad1 */
+ 
      }
  
      if (ParameterCount > 0) {
***************
*** 8590,8609 ****
        offset += ParameterCount; /* Skip Parameters */
  
      }
  
!     if (offset % 2) {
  	
        /* Build display for: Pad2 */
  
-       Pad2 = GBYTE(pd, offset);
- 
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, 1, "Pad2: %u", Pad2);
  
        }
  
!       offset += 1; /* Skip Pad2 */
  
      }
  
--- 8587,8606 ----
        offset += ParameterCount; /* Skip Parameters */
  
      }
+ 
+     if (DataCount > 0 && offset < (SMB_offset + DataOffset)) {
  
!       int pad2Count = SMB_offset + DataOffset - offset;
  	
        /* Build display for: Pad2 */
  
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, pad2Count, "Pad2: %s", format_text(pd + offset, pad2Count));
  
        }
  
!       offset += pad2Count; /* Skip Pad2 */
  
      }
  
***************
*** 8778,8794 ****
  
      offset += 1; /* Skip Reserved3 */
  
!     /* Build display for: Setup */
  
!     Setup = GSHORT(pd, offset);
  
!     if (tree) {
  
!       proto_tree_add_text(tree, NullTVB, offset, 2, "Setup: %u", Setup);
  
!     }
  
!     offset += 2; /* Skip Setup */
  
      /* Build display for: Byte Count (BCC) */
  
--- 8775,8800 ----
  
      offset += 1; /* Skip Reserved3 */
  
!     if (SetupCount > 0) {
  
!       int i = SetupCount;
  
!       Setup = GSHORT(pd, offset);
  
!       for (i = 1; i <= SetupCount; i++) {
! 	
! 	Setup = GSHORT(pd, offset);
  
! 	if (tree) {
! 
! 	  proto_tree_add_text(tree, NullTVB, offset, 2, "Setup%i: %u", i, Setup);
! 
! 	}
! 
! 	offset += 2; /* Skip Setup */
  
!       }
!     }
  
      /* Build display for: Byte Count (BCC) */
  
***************
*** 8802,8818 ****
  
      offset += 2; /* Skip Byte Count (BCC) */
  
!     /* Build display for: Pad1 */
  
!     Pad1 = GBYTE(pd, offset);
  
!     if (tree) {
  
!       proto_tree_add_text(tree, NullTVB, offset, 1, "Pad1: %u", Pad1);
  
!     }
  
!     offset += 1; /* Skip Pad1 */
  
      /* Build display for: Parameter */
  
--- 8808,8827 ----
  
      offset += 2; /* Skip Byte Count (BCC) */
  
!     if (offset < (SMB_offset + ParameterOffset)) {
  
!       int pad1Count = SMB_offset + ParameterOffset - offset;
  
!       /* Build display for: Pad1 */
  
!       if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, pad1Count, "Pad1: %s", format_text(pd + offset, pad1Count));
!       }
! 
!       offset += pad1Count; /* Skip Pad1 */
  
!     }
  
      /* Build display for: Parameter */
  
***************
*** 8828,8844 ****
  
      }
  
!     /* Build display for: Pad2 */
  
!     Pad2 = GBYTE(pd, offset);
  
!     if (tree) {
  
!       proto_tree_add_text(tree, NullTVB, offset, 1, "Pad2: %u", Pad2);
  
!     }
  
!     offset += 1; /* Skip Pad2 */
  
      /* Build display for: Data */
  
--- 8837,8857 ----
  
      }
  
!     if (DataCount > 0 && offset < (SMB_offset + DataOffset)) {
  
!       int pad2Count = SMB_offset + DataOffset - offset;
! 	
!       /* Build display for: Pad2 */
  
!       if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, pad2Count, "Pad2: %s", format_text(pd + offset, pad2Count));
  
!       }
! 
!       offset += pad2Count; /* Skip Pad2 */
  
!     }
  
      /* Build display for: Data */
  
***************
*** 8865,8871 ****
    char             *TransactNameCopy;
    char             *trans_type = NULL, *trans_cmd, *loc_of_slash = NULL;
    int              index;
-   guint8           Pad2;
    const gchar      *Data;
  
    if (!TransactName)
--- 8878,8883 ----
***************
*** 8908,8927 ****
        offset = SMB_offset + ParameterOffset + ParameterCount; /* Skip Parameters */
  
      }
  
!     if (offset % 2) {
  	
        /* Build display for: Pad2 */
  
-       Pad2 = GBYTE(pd, offset);
- 
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, 1, "Pad2: %u: %u", Pad2, offset);
  
        }
  
!       offset += 1; /* Skip Pad2 */
  
      }
  
--- 8920,8939 ----
        offset = SMB_offset + ParameterOffset + ParameterCount; /* Skip Parameters */
  
      }
+ 
+     if (DataCount > 0 && offset < (SMB_offset + DataOffset)) {
  
!       int pad2Count = SMB_offset + DataOffset - offset;
  	
        /* Build display for: Pad2 */
  
        if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, pad2Count, "Pad2: %s", format_text(pd + offset, pad2Count));
  
        }
  
!       offset += pad2Count; /* Skip Pad2 */
  
      }
  
***************
*** 8954,8960 ****
    guint8        SetupCount;
    guint8        Reserved3;
    guint8        Reserved1;
-   guint8        Pad1;
    guint8        MaxSetupCount;
    guint32       Timeout;
    guint16       TotalParameterCount;
--- 8966,8971 ----
***************
*** 9297,9315 ****
      offset += TNlen; /* Skip Transact Name */
      if (si.unicode) offset += 2;   /* There are two more extraneous bytes there*/
  
!     if (offset % 2) {
  
!       /* Build display for: Pad1 */
  
!       Pad1 = GBYTE(pd, offset);
  
        if (tree) {
- 
- 	proto_tree_add_text(tree, NullTVB, offset, 1, "Pad1: %u", Pad1);
  
        }
!     
!       offset += 1; /* Skip Pad1 */
  
      }
  
--- 9308,9325 ----
      offset += TNlen; /* Skip Transact Name */
      if (si.unicode) offset += 2;   /* There are two more extraneous bytes there*/
  
!     if (offset < (SMB_offset + ParameterOffset)) {
  
!       int pad1Count = SMB_offset + ParameterOffset - offset;
  
!       /* Build display for: Pad1 */
  
        if (tree) {
  
+ 	proto_tree_add_text(tree, NullTVB, offset, pad1Count, "Pad1: %s", format_text(pd + offset, pad1Count));
        }
! 
!       offset += pad1Count; /* Skip Pad1 */
  
      }
  
***************
*** 9480,9496 ****
  
      if (SetupCount > 0) {
  
!       /* Hmmm, should code for all setup words ... */
  
        Setup = GSHORT(pd, offset);
  
!       if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, 2, "Setup: %u", Setup);
  
!       }
  
!     offset += 2; /* Skip Setup */
  
      }
  
--- 9490,9512 ----
  
      if (SetupCount > 0) {
  
!       int i = SetupCount;
  
        Setup = GSHORT(pd, offset);
  
!       for (i = 1; i <= SetupCount; i++) {
! 	
! 	Setup = GSHORT(pd, offset);
  
! 	if (tree) {
  
! 	  proto_tree_add_text(tree, NullTVB, offset, 2, "Setup%i: %u", i, Setup);
  
! 	}
! 
! 	offset += 2; /* Skip Setup */
! 
!       }
  
      }
  
***************
*** 9508,9524 ****
  
      /* Build display for: Pad1 */
  
!     if (offset % 2) {
  
!       Pad1 = GBYTE(pd, offset);
  
!       if (tree) {
  
! 	proto_tree_add_text(tree, NullTVB, offset, 1, "Pad1: %u", Pad1);
  
        }
  
!       offset += 1; /* Skip Pad1 */
  
      }
  
--- 9524,9541 ----
  
      /* Build display for: Pad1 */
  
!     if (offset < (SMB_offset + ParameterOffset)) {
  
!       int pad1Count = SMB_offset + ParameterOffset - offset;
  
!       /* Build display for: Pad1 */
  
!       if (tree) {
  
+ 	proto_tree_add_text(tree, NullTVB, offset, pad1Count, "Pad1: %s", format_text(pd + offset, pad1Count));
        }
  
!       offset += pad1Count; /* Skip Pad1 */
  
      }