On Jun 10, 2008, at 1:07 PM, david_aggeler@xxxxxxxxxx wrote:
In order to better help the end user reading captures, I'm trying to
set hints, when decoding problems arise, due to packet data.
My first attempt was using DISSECTOR_ASSERT(), but this causes a
'bug in dissector'. After reading a thread the dev-archive, this is
apparently by design, but about 50% of the DISSECTOR_ASSERT() code
lines I came across are to catch somewhat expected data errors.
If that's true, that's a bug.
DISSECTOR_ASSERT() is to use to catch dissector *bugs*, i.e. stuff
that "shouldn't happen" no matter *what* problems there are in the
data, as per
http://www.wireshark.org/lists/wireshark-dev/200704/msg00233.html
"You should never call DISSECTOR_ASSERT() for bad packet data.
DISSECTOR_ASSERT() should only be used to detect bad dissector
code(e.g. causing an endless recursion)."
and
http://www.wireshark.org/lists/wireshark-dev/200704/msg00234.html
"You're correct - neither g_assert() nor DISSECTOR_ASSERT() are
appropriate for problems with packet data. An assert is something that
should be used to test something that should *always* be true - i.e.,
there's an error in the code if it's not true. That applies to
assert(), g_assert(), and DISSECTOR_ASSERT() - and to any
DISSECTOR_ASSERT_... macro."
As suggested there, I switched to expert_add_info, but with the
result, that my packets are not marked MALFORMED, which I think is a
pity.
The "malformed" indication originally referred only to packets that
were too short, so that the dissector, when trying to dissect a field
that is supposed to be there, or a variable-length field that has a
specified length, or..., gets an exception thrown by the tvbuff code.
There is now also the PI_MALFORMED expert information group. The two
are, confusingly, not connected. There should probably be a way to
look for various expert info indications with a display filter - and
that could perhaps replace the existing "malformed" "protocol". The
various exceptions that would cause a "malformed" indication could
enter an item into the protocol tree with the expert info in question
attached to it.
- Is there a 'best practice' to MALFORMED PACKETS without 'bug in
dissector'?
There's a *current* practice, which is to lookup the "malformed"
protocol and put an entry into the protocol tree for that "protocol".
Whether that's the *best* practice is another matter.
- In case nobody is working on 'DISSECTOR_VERIFY_DATA' yet, I'm
willing to contribute code, but so far only worked on a dissector
for a while. In addition I am not really an exception handler
specialist, and I guess, this is pretty core code. A rough hint of
what would need to be done could maybe get me started.
Note that not all protocol problems should throw an exception; for
example, if a protocol has an "XYZZY" TLV, which always has a 4-byte
integer value, but the packet has an XYZZY TLV with a value length of
8, that should be reported as a malformed TLV, *but* the dissector can
skip that TLV (as it has the length) and keep dissecting TLVs. Thus,
DISSECTOR_VERIFY_DATA() shouldn't *always* be what's used to report
packet problems; it should only be used to report packet problems that
prevent any further dissection of the packet.
