Ethereal-dev: Re: [Ethereal-dev] Should alloc_field_info() check for start value inside tvb?
Ulf Lamping wrote:
As looking at the recent buildbot problems, and the changes you've done
Guy, I'm asking myself if the function alloc_field_info(proto.c 2006)
*always* should check if the start parameter is inside the tvb.
Or are there any circumstances, where this is intentionally *not* the case?
All the routines that add protocol tree items should, at minimum, check
whether the range of bytes referred to by the start and length arguments
is inside the tvbuff.
alloc_field_info() is an internal routine, so that check could be done
by its callers in some cases - for example, if proto_tree_add_item() is
being called, a check by alloc_field_info() would be redundant.
Another question, is this a:
- [Malformed Packet] "tvb_ensure_length_remaining(tvb, start)" or a
- [Dissector Bug] "DISSECTOR_ASSERT(tvb_reported_length_remaining(tvb,
start) != -1)"?
I would tend to use [Malformed Packet], just like an invalid length
parameter.
I'd say that if the offset is immediately past the end of the tvbuff,
it's whatever exception is appropriate - Short Frame or Malformed
Packet; tvb_ensure_length_remaining() should ensure that.
I'm not certain about the other cases, but given that a dissector might
just skip padding fields, those should probably also throw BoundsError
or ReportedBoundsError as appropriate.
