osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

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
This commit is contained in:
Bill Meier 2009-07-03 16:03:02 +00:00
parent 1415c62c68
commit d36f35f28b
2 changed files with 12 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
epan/dissectors/packet-radius.c
View File

@ -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);
}

10
epan/radius_dict.l
View File

@ -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);