[tcpdump-workers] CVS radius code seems flawed

[Jonathan Heusser]

Hello again,

The following bugs concern _only_ the CVS version of tcpdump!
The code in print-radius.c, introduced with rcs version 1.24 seems 
heavily flawed.

Function print_vendor_attr():

   * At print-radius.c:509, the while() loop does not check the data 
pointer for termination.

Function radius_attr_print():

   * On line 808 and 815 rad_attr->type is unchecked and used as an 
array index. This leads to an array out of bound
access if you send a crafted packet with a big type field.

   * Line 838 print_unknown_data() is called without checking a minimum 
size of rad_attr->len.

All these vulnerabilities lead to segmentation faults and of course to 
possible exploitation.
Tested against tcpdump-2004.01.21.

A possible patch is attached.

Thanks,
Jonathan Heusser
--
Key fingerprint = 2A55 EB7C B7EA 6336 7767  4A47 910A 307B 1333 BD6C
--- print-radius.c.old  Thu Jan 22 23:07:10 2004
+++ print-radius.c      Thu Jan 22 23:02:02 2004
@@ -506,7 +506,7 @@
            tok2str(oui_values,"Unknown",vendor_id),
            vendor_id);
 
-    while (length >= 2) {
+    while (length >= 2 && *data) {
         vendor_type = *(data);
         vendor_length = *(data+1);
         data+=2;
@@ -803,14 +803,14 @@
 
    while (length > 0)
    {
-     if (rad_attr->len == 0)
+     if (rad_attr->len == 0 && rad_attr->type < (TAM_SIZE(attr_type)-1))
      {
        printf("\n\t  %s Attribute (%u), zero-length",
                attr_type[rad_attr->type].name,
                rad_attr->type);
        return;
      }
-     if ( rad_attr->len <= length )
+     if ( rad_attr->len <= length && rad_attr->type < (TAM_SIZE(attr_type)-1))
      {
          printf("\n\t  %s Attribute (%u), length: %u, Value: ",
                 attr_type[rad_attr->type].name,
@@ -834,7 +834,7 @@
         return;
      }
      /* do we want to see an additionally hexdump ? */
-     if (vflag> 1)
+     if (vflag> 1 && rad_attr->len >= 2)
          print_unknown_data((char *)rad_attr+2,"\n\t    ",(rad_attr->len)-2);
 
      length-=(rad_attr->len);