Wireshark · Ethereal-dev: Re: [Ethereal-dev] Bugs in osi_utils.c

[Guy Harris <guy@xxxxxxxxxx>]

> I've just found a couple of bugs in osi_utils.c in the routines :
> 
> print_system_id
> print_area
> 
> there is the line
> 
> cur += sprintf( str, "%02x%02x%02x%02x.", buffer[tmp++],
> buffer[tmp++],buffer[tmp++], buffer[tmp++] );

Which also has *another* bug - the first argument to "sprintf()" should
be "cur", not "str", otherwise it overwrites the string with the hex
values of the next four octets, rather than appending them.

> This however does not work due to tmp++ being a post increment and occuring
> AFTER the sprintf has
> took place. This means you get the first byte replicated in the generated string
>  all four times.
> The simple fix I have made on ours for now is splitting it over four lines ,
> thus
> 
>         cur += sprintf(str,"%02x",buffer[tmp++]);
>         cur += sprintf(cur,"%02x",buffer[tmp++]);
>         cur += sprintf(cur,"%02x",buffer[tmp++]);
>         cur += sprintf(cur,"%02x",buffer[tmp++]);

Actually, the first line needs to be

        cur += sprintf(cur,"%02x",buffer[tmp++]);

as per the other bug, and the last line needs to be

        cur += sprintf(cur,"%02x.",buffer[tmp++]);

to put the "." at the end.

I've attached to my mail the patch for that change, which I've checked
in.

? errs
? mtu
? .pure
? Makefile.gprof
? WORK
? gtkdialogbox.c
? gtkdialogbox.h
? errs.DU
? ethereal.hpux
? errs.hpux
? packet-smb-pipe.c.NEW
? packet-sual.c.diff
? capture.c.WARNING
? simple_dialog.h.WARNING
? epan/errs
? gtk/simple_dialog.c.WARNING
? wiretap/SAVE
? wiretap/filter-fddi
? wiretap/Makefile.gprof
? wiretap/.pure
Index: epan/osi-utils.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/epan/osi-utils.c,v
retrieving revision 1.3
diff -c -r1.3 osi-utils.c
*** osi-utils.c	2001/04/16 10:04:33	1.3
--- osi-utils.c	2001/05/15 18:58:03
***************
*** 86,92 ****
   
    cur = str;
    if ( ( 6 == length ) || ( 7 == length ) ) { /* Special case, print as MAC */
!     cur += sprintf(str, "%02x%02x.%02x%02x.%02x%02x", buffer[0], buffer[1],
                      buffer[2], buffer[3], buffer[4], buffer[5] );
      if ( 7 == length ) {
        sprintf( cur, "-%02x", buffer[6] );
--- 86,92 ----
   
    cur = str;
    if ( ( 6 == length ) || ( 7 == length ) ) { /* Special case, print as MAC */
!     cur += sprintf(cur, "%02x%02x.%02x%02x.%02x%02x", buffer[0], buffer[1],
                      buffer[2], buffer[3], buffer[4], buffer[5] );
      if ( 7 == length ) {
        sprintf( cur, "-%02x", buffer[6] );
***************
*** 95,102 ****
    else {
      tmp = 0;
      while ( tmp < length / 4 ) { /* 16 / 4 == 4 > four Octets left to print */
!       cur += sprintf( str, "%02x%02x%02x%02x.", buffer[tmp++], buffer[tmp++],
!                       buffer[tmp++], buffer[tmp++] );
      }
      if ( 1 == tmp ) {   /* Special case for Designated IS */
        sprintf( --cur, "-%02x", buffer[tmp] );
--- 95,104 ----
    else {
      tmp = 0;
      while ( tmp < length / 4 ) { /* 16 / 4 == 4 > four Octets left to print */
!       cur += sprintf( cur, "%02x", buffer[tmp++] );
!       cur += sprintf( cur, "%02x", buffer[tmp++] );
!       cur += sprintf( cur, "%02x", buffer[tmp++] );
!       cur += sprintf( cur, "%02x.", buffer[tmp++] );
      }
      if ( 1 == tmp ) {   /* Special case for Designated IS */
        sprintf( --cur, "-%02x", buffer[tmp] );
***************
*** 158,166 ****
  			return( str );
         }
      if ( 4 < length ) { 
!       while ( tmp < length / 4 ) {      /* 16/4==4  four Octets left to print */
!         cur += sprintf( str, "%02x%02x%02x%02x.", buffer[tmp++], buffer[tmp++],
!                         buffer[tmp++], buffer[tmp++] );
        }
        if ( 1 == tmp ) {                     /* Special case for Designated IS */
          sprintf( --cur, "-%02x", buffer[tmp] );
--- 160,170 ----
  			return( str );
         }
      if ( 4 < length ) { 
!       while ( tmp < length / 4 ) {      /* 16/4==4 > four Octets left to print */
!         cur += sprintf( cur, "%02x", buffer[tmp++] );
!         cur += sprintf( cur, "%02x", buffer[tmp++] );
!         cur += sprintf( cur, "%02x", buffer[tmp++] );
!         cur += sprintf( cur, "%02x.", buffer[tmp++] );
        }
        if ( 1 == tmp ) {                     /* Special case for Designated IS */
          sprintf( --cur, "-%02x", buffer[tmp] );