Ethereal-dev: Re: [Ethereal-dev] ethereal crash on particular packet

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

From: Guy Harris <gharris@xxxxxxxxx>
Date: Fri, 14 Jun 2002 00:47:24 -0700
On Thu, Jun 13, 2002 at 04:28:42PM +0200, Andreas Ferber wrote:
> It looks like something is trashing data on the stack

Probably.

I can't reproduce it on my machine by reading that particular file, but
the bogus address in

	Cannot access memory at address 0x3631785b

looks like the string [x16, which would be part of a string printed for
RFC 2673 bitstring labels in DNS, and with a really large bit count that
string could overflow a buffer.

The attached patch *should* prevent that; get the 0.9.4 source, apply
the patch to "packet-dns.c", rebuild, and try to run the resulting
Ethereal on the capture file that caused the crash.

> The crash occurs with both ethereal 0.8.20 and 0.9.4, and affects both
> the GUI and the commandline version. editcap doesn't crash on the
> packet, so the problem seems to be somewhere in the packet decoding
> code. The trace was captured originally with tcpdump, so I can't tell
> if it would crash ethereal while only capturing into a file without
> showing decoded data.

It probably wouldn't crash Ethereal or Tethereal if you're only
capturing to a file.
Index: packet-dns.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-dns.c,v
retrieving revision 1.87
diff -c -r1.87 packet-dns.c
*** packet-dns.c	15 May 2002 07:24:20 -0000	1.87
--- packet-dns.c	14 Jun 2002 07:43:32 -0000
***************
*** 536,552 ****
  	{
  	  int bit_count;
  	  int label_len;
  
  	  bit_count = tvb_get_guint8(tvb, offset);
  	  offset++;
  	  label_len = (bit_count - 1) / 8 + 1;
  	
! 	  np += sprintf(np, "\\[x");
  	  while(label_len--) {
! 	    np += sprintf(np, "%02x", tvb_get_guint8(tvb, offset));
  	    offset++;
  	  }
! 	  np += sprintf(np, "/%d]", bit_count);
  	}
  	break;
  
--- 536,590 ----
  	{
  	  int bit_count;
  	  int label_len;
+ 	  int print_len;
  
  	  bit_count = tvb_get_guint8(tvb, offset);
  	  offset++;
  	  label_len = (bit_count - 1) / 8 + 1;
  	
! 	  if (maxname > 0) {
! 	    print_len = snprintf(np, maxname + 1, "\\[x");
! 	    if (print_len != -1) {
! 	      /* Some versions of snprintf return -1 if they'd truncate
! 	         the output. */
! 	      np += print_len;
! 	      maxname -= print_len;
! 	    } else {
! 	      /* Nothing printed, as there's no room.
! 	         Suppress all subsequent printing. */
! 	      maxname = 0;
! 	    }
! 	  }
  	  while(label_len--) {
! 	    if (maxname > 0) {
! 	      print_len = snprintf(np, maxname + 1, "%02x",
! 	        tvb_get_guint8(tvb, offset));
! 	      if (print_len != -1) {
! 		/* Some versions of snprintf return -1 if they'd truncate
! 		 the output. */
! 		np += print_len;
! 		maxname -= print_len;
! 	      } else {
! 		/* Nothing printed, as there's no room.
! 		   Suppress all subsequent printing. */
! 		maxname = 0;
! 	      }
! 	    }
  	    offset++;
  	  }
! 	  if (maxname > 0) {
! 	    print_len = snprintf(np, maxname + 1, "/%d]", bit_count);
! 	    if (print_len != -1) {
! 	      /* Some versions of snprintf return -1 if they'd truncate
! 	         the output. */
! 	      np += print_len;
! 	      maxname -= print_len;
! 	    } else {
! 	      /* Nothing printed, as there's no room.
! 	         Suppress all subsequent printing. */
! 	      maxname = 0;
! 	    }
! 	  }
  	}
  	break;