Wireshark · Ethereal-dev: Re: [ethereal-dev] Re: [ethereal-users] kde locks up....with radius..

[Gilbert Ramirez <gram@xxxxxxxxxx>]

On Thu, Dec 02, 1999 at 04:39:05PM -0600, Arni Raghu wrote:
> 
> 
> Hi,
> Here is the dump file...attached in binary format..
> 
> I just looked at th source and saw the radius part...silly I should have
> seen that b4...
> 
> Hope u can fix the soon..appreciate the help

Yes, there was an infinite loop in packet-radius.c.

1. It wasn't checking for short packets. Your trace is only
capturing the first 68 bytes, which sounds like you're capturing
with tcpdump without using the "-s" option. I recommend either
capturing with ethereal (which has bigger default), or use
"-s" with tcpdump. You're capturing on loopback, you'll have to
check to see what your MTU is. On my loopback, it's 3924 bytes.

2. The radius code was using a value from the packet data (or
in this case, from random memory, because of the short packet)
as a loop decrementer. It wasn't checking to see if it had decremented
that loop by 0. So, it spun into an infinite loop.

I wish C had a "tainted" mode like Perl, where we could say "do not
trust any data received from a packet unless we explicitly untaint
it".

Here's the diff that I'm checking into Ethereal's CVS.

--gilbert

Index: packet-radius.c
===================================================================
RCS file: /usr/local/cvsroot/ethereal/packet-radius.c,v
retrieving revision 1.5
diff -u -r1.5 packet-radius.c
--- packet-radius.c	1999/11/16 11:42:49	1.5
+++ packet-radius.c	1999/12/02 23:18:53
@@ -529,10 +529,16 @@
      avptpstrval=match_strval(avph.avp_type, radius_attrib_type_vals);
      if (avptpstrval == NULL) avptpstrval="Unknown Type";
      valstr=rd_value_to_str(&avph, pd, offset);
+     if (!BYTES_ARE_IN_FRAME(offset, avph.avp_length)) {
+	break;
+     }
      proto_tree_add_text(tree,offset,avph.avp_length,
         "t:%s(%d) l:%d, value:%s",
         avptpstrval,avph.avp_type,avph.avp_length,valstr);
      offset=offset+avph.avp_length;
+     if (avph.avp_length == 0) {
+	break;
+     }
   }
 }