From d36f35f28b7f56b80bbfdd4642720029b5103370 Mon Sep 17 00:00:00 2001 From: Bill Meier Date: Fri, 3 Jul 2009 16:03:02 +0000 Subject: From: Tobias Witek: Fix processing of radius 'vendor specific attributes' In the RADIUS dissector, the function radius_register_avp_dissector() registers vendors that are not already present in the dictionary hash-table. As far as I can see, there are two problems with this: 1. The function does not set the number of type/length octets and the has_flags variable for that AVP, which is required to correctly decode AVP/VSA values 2. In some situations, the function is called _before_ radius_load_dictionary() is called (for example for the vendor 3GPP (ID: 10415)) Therefore, all vendor entries that are created by calling radius_register_avp_dissector() leave their type_octets and length_octets un-initialized, which causes incorrect decoding. [Result: Radius dissector displays messages such as: "Malformed Packet: RADIUS" and "Error/Malformed: Malformed Packet(Exception occurred)"] The attached patch fixes this problem by assuming that the dictionary knows the 'ground truth' about the type/length octet and the has_flags information and allows it to overwrite these values even for vendors that have already been loaded. Also: (from Bill Meier): set the type/length octet and the has_flags variables to default "standard" values (1,1,FALSE) in radius_register_avp_dissector(). Fixes Bug #3651 (and Bug #3635). https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3651 https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=3635 svn path=/trunk/; revision=28937 --- epan/dissectors/packet-radius.c | 5 +++++ epan/radius_dict.l | 10 +++++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/epan/dissectors/packet-radius.c b/epan/dissectors/packet-radius.c index 23ac77785a..e2e8a09456 100644 --- a/epan/dissectors/packet-radius.c +++ b/epan/dissectors/packet-radius.c @@ -1780,6 +1780,11 @@ extern void radius_register_avp_dissector(guint32 vendor_id, guint32 attribute_i vendor->attrs_by_id = g_hash_table_new(g_direct_hash,g_direct_equal); vendor->ett = no_vendor.ett; + /* XXX: Default "standard" values: Should be parameters ? */ + vendor->type_octets = 1; + vendor->length_octets = 1; + vendor->has_flags = FALSE; + g_hash_table_insert(dict->vendors_by_id,GUINT_TO_POINTER(vendor->code),vendor); g_hash_table_insert(dict->vendors_by_name,(gpointer)(vendor->name),vendor); } diff --git a/epan/radius_dict.l b/epan/radius_dict.l index 6d7337e4cd..719b752278 100644 --- a/epan/radius_dict.l +++ b/epan/radius_dict.l @@ -305,10 +305,14 @@ void add_vendor(const gchar* name, guint32 vendor_id, guint vendor_type_octets, v->code = vendor_id; v->ett = -1; v->name = NULL; - v->type_octets = vendor_type_octets; - v->length_octets = vendor_length_octets; - v->has_flags = vendor_has_flags; } + /* Assume that the dictionary knows the 'ground truth' about the + * type/length/has_flags information and thus allow the dictionary to + * overwrite these values even for vendors that have already been loaded. + */ + v->type_octets = vendor_type_octets; + v->length_octets = vendor_length_octets; + v->has_flags = vendor_has_flags; if (v->name) g_free((gpointer) v->name); -- cgit v1.2.3