Wireshark-bugs: [Wireshark-bugs] [Bug 10329] Probably wrong length check in proto_item_set_end

Date Prev · Date Next · Thread Prev · Thread Next
Date: Fri, 01 Aug 2014 01:02:53 +0000

changed bug 10329

What Removed Added
Status UNCONFIRMED CONFIRMED
CC   [email protected]
Ever confirmed   1

Comment # 2 on bug 10329 from
Huh, this is an interesting one. The assertion inside proto_item_set_end is
absolutely correct, we're just doing something weird.

All of the following is referring to 1.12 because that's where my
plugin-capable build environment is right now. Should be effectively the same
on master with different line numbers. The error only occurs when tree is
non-NULL but hidden (best achieved with "-Y frame" to tshark).

ANALYSIS

At opcua.c:416 in frame 15 we pass in three values to parseService:
 - a tree whose underlying item was added to the "main" tvb
 - a (reassembled) child tvb
 - an offset of 0

At opcua_transport_layer.c:193 we add a tree item (proto_tree_add_text), which
short-circuits and returns the input tree due to TRY_TO_FAKE_THIS_ITEM noticing
the tree is not visible.

At opcua_transport_layer.c:196 we call proto_item_set_end on this tree item,
with an offset of 4, on the (reasonable) assumption that it was created with an
offset of 0 four lines previously. However, because _add_text short-circuited
and returned the input tree, and because the input tree was added on a totally
different TVB with an unrelated offset, the assertion makes no sense for this
data and blows up.

PROBLEM

The root of the problem is that the 'start' value of the returned tree at line
193 (ti_inner) bears no relation to the starting offset actually passed into
the proto_tree_add_text call. We pass in an offset of 0 and it gives us back a
tree item "starting" at 66.

SOLUTION

Have TRY_TO_FAKE_THIS_ITEM compare PTREE_FINFO(tree)->ds_tvb with the TVB
passed into the caller, and *not* short-circuit if they're different (which
should be rare).

I would appreciate somebody else verifying this makes sense...


You are receiving this mail because:
  • You are watching all bug changes.