This is a static archive of our old Q&A Site. Please post any new questions and answers at ask.wireshark.org.

display filter in RADIUS sub-dissector crashes

0

Hi,

I am working on proprietary RADIUS protocol dissection. I have created 3 sub-dissectors within wireshark which are called from packet-radius.c

I want to add one custom display filter in sub-dissector.

I am using call_dissector_with_data API in dissect_radius to call my sub-dissector. I am passing tree pointer in this API.

When I use proto_tree_add_uint API by passing this tree pointer and appropriate 32-bit value, wireshark is getting crashed with an error message,

Unhandled exception at 0x00007FF94DED4B07 (libwireshark.dll) in Wireshark.exe: 0xC0000005: Access violation reading location 0xFFFFFFFFFFFFFFFF.

In packet-radius.c,

rad_handle = find_dissector("radius_display");

if (rad_handle ) {


/* Call to sub-dissector */
call_dissector_with_data(rad_handle , tvb, pinfo, tree, NULL);



}

In packet-radius-display.c,

static int hf_radius_resp_time = -1;


dissect_radius_display(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data U)
{
guint32 resptime = 0;
proto_item *item;


resptime = calc_resp_time(pinfo); // custom function to calculate response time

    if (tree) {

    item = proto_tree_add_uint(tree, hf_radius_resp_time, NULL, 0, 0, resptime);
    PROTO_ITEM_SET_HIDDEN(item);
}



}


void proto_register_radius_display(void)
{
int proto_radius_display = -1;


hf_register_info hf[] = {
    { &hf_radius_resp_time,
    { "Response Time", "radius.resptime", FT_UINT32, BASE_DEC, NULL, 0,
    "The time between the request and the response, in ms", HFILL } }
};

proto_radius_display = proto_register_protocol("RADIUS Protocol Display", "RADIUS Display", "radius_display");

proto_register_field_array(proto_radius_display, hf, array_length(hf));

register_dissector("radius_display", dissect_radius_display, proto_radius_display);



}

asked 17 Jul ‘17, 07:07

Mehul28's gravatar image

Mehul28
0●4●5●8
accept rate: 0%

edited 17 Jul ‘17, 22:45

We need to see an abstract of your code to see what’s wrong.

(17 Jul ‘17, 11:47) Anders ♦

Ok. I have added the code snippet.

(17 Jul ‘17, 22:46) Mehul28

Sure it’s not crashing in calc_resp_time() ?

(21 Jul ‘17, 02:27) Anders ♦

No. calc_resp_time is a function which simply does some arithmetic operation. Wireshark is getting crashed on proto_tree_add_uint call…

(21 Jul ‘17, 02:41) Mehul28

So if you know that it’s crashing in proto_tree_add_uint(), you have a stack trace showing proto_tree_add_uint(); what does the stack trace say?

(21 Jul ‘17, 21:49) Guy Harris ♦♦

2 Answers:

0

proto_tree_add_uint() may not be used with tvb = NULL and length = 0 from what I can see.

answered 18 Jul '17, 02:02

Anders's gravatar image

Anders ♦
4.6k●9●52
accept rate: 17%

One can use this, for just developing display filter. There are other packet dissections which have done the same in wireshark.

The following code works,

   item = proto_tree_add_uint(tree, hf_radius_resp_time, NULL, 0, 0, resptime);
   PROTO_ITEM_SET_HIDDEN(item)

whien I insert it in dissect_radius function of packet-radius.c

(18 Jul '17, 02:08) Mehul28

I don't know why it's getting crashed when I use the proto_tree_add_uint API with radius_tree in my sub-dissector?

(21 Jul '17, 01:53) Mehul28

What does your debugger say?

(21 Jul '17, 02:19) Jaap ♦

Access violation reading location 0xFFFFFFFFFFFFFFFF.

(21 Jul '17, 02:42) Mehul28

That looks like a -1 to me, are you absolutely sure that hf_radius_resp_time is being initialised by proto_register_radius_display()? Is the latter being called?

Also, int proto_radius_display = -1; should really be a static int int proto_radius_display = -1; at file level, not in proto_register_radius_display().

(21 Jul '17, 08:24) grahamb ♦

proto_tree_add_uint() may not be used with tvb = NULL and length = 0 from what I can see.

If you're referring to the line

DISSECTOR_ASSERT(tvb != NULL || *length == 0);

in get_hfi_length(), that says that it may not be used with tvb = NULL and length not equal to 0; a null tvb pointer is ok IF the length is 0.

So

item = proto_tree_add_uint(tree, hf_radius_resp_time, NULL, 0, 0, resptime);

shouldn't crash with that assertion failure.

(21 Jul '17, 21:36) Guy Harris ♦♦
showing 5 of 6 show 1 more comments

0

Hi,

This is resolved by registering filter variable hf_radius_resp_time using wmem and epan_scope.

hf = wmem_array_new(wmem_epan_scope(), sizeof(hf_register_info));

Previously, I was registering it statically.

Thanks

answered 03 Aug '17, 05:05

Mehul28's gravatar image

Mehul28
0●4●5●8
accept rate: 0%

edited 03 Aug '17, 05:06

@Mehul28, this site has a mechanism of marking questions as usefully answered. If an Answer has usefully answered a Question, the author of the Question (and nobody else) has the possibility to mark it as the correct answer by clicking the checkmark icon next to it. By doing so you change the colour of the Question in the list to green, indicating it as usefully answered to others. There is nothing wrong about marking your own Answer as the correct one.

(03 Aug '17, 06:12) sindy

registering filter variable hf_radius_resp_time using wmem and epan_scope.

Presumably you mean "allocating filter variable..." - you register it in the proto_register_field_array() call.

Are you modifying hf after you initialize it? If not, then it shouldn't matter where it's located in the address space, so it shouldn't matter whether you allocate it statically or dynamically.

(03 Aug '17, 10:51) Guy Harris ♦♦