Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 2402] Data string filter crash

[bugzilla-daemon@xxxxxxxxxxxxx]

http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2402


Stig Bjørlykke <stig@xxxxxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |stig@xxxxxxxxxxxxx
           Priority|Medium                      |High




--- Comment #1 from Stig Bjørlykke <stig@xxxxxxxxxxxxx>  2008-04-21 13:23:10 GMT ---
Thank you for reporting this problem!  When searching for the error I found
more potential errors, which I will try to describe here in more details.  I
have a solution for this particular error, but not yet for the general problem.


This particular error occurs because we have two dissectors registering the
"data" field with different types.  packet-data has this as a FT_PROTOCOL and
packet-tpncp has this as FT_STRING.

When _validating_ the given display filter (data eq "string") we use the latest
registered type, in this case from TPNCP which is a FT_STRING.  The string type
has defined the "eq" operator (a function to handle "eq", found in
ftype-string.c) and the display filter is marked as valid.  The second argument
to the "eq" operator is converted to a string.

When _using_ this filter we are using the registered type from the matching
field from the packet, in this case from DATA which is a FT_PROTOCOL.  The
protocol type has not defined a function to handle the "eq" operator (a NULL
pointer) and we fall into the g_assign in fvalue_eq().  The first argument to
the "eq" operator is converted to a tvbuff.

This looks like a design flaw, as we currently does not detect these
situations.


Another example:
We have this two elements, both with the same abbrew.  Having the display
filter "elem eq 3" leads to a crash if we have elem1 in the packet tree,
because we are handling 3 as a pointer.

  { &hf_elem1,
    { "Elem 1", "elem", FT_STRING, BASE_HEX, 
      NULL, 0x00, NULL, HFILL}},
  { &hf_elem2, 
    { "Elem 2", "elem", FT_UINT8, BASE_DEC, 
      NULL, 0x00, NULL, HFILL } },


I have been thinking about the following solutions:

1. Rewrite TPNCP to add "tpncp." before all filter names (easy)
This will fix this particular error, but will not fix other problems that may
occur.

2. Implement all the operators in ftype-tvbuff.c (easy)
This allows us to do more filtering on FT_PROTOCOL types (which is probably not
what the user wants), but does not solve this problem because the two arguments
to the "eq" function has different types, here tvbuff vs. string.

3. Disallow fields with same names and different types. (not an option?)
This will break some of our dissectors, but may be easy to implement.  I think
this shall be possible tho, because we have a linked list with different fields
and the same name.

4. Make all "cmp_*" functions handle different types for the second argument
(not clean)
Probably not an optimal solution.

5. Convert the second argument according to the type of the first argument
(don't know)
This is maybe the most optimal solution, but I don't know the internals so I
have no idea how much work or if this will lead to performance loss.


I really don't know how many such problems we have in Wireshark today...


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.