Ethereal-dev: Re: [Ethereal-dev] [PATCH] SNA Update

Note: This archive is from the project's previous web site, ethereal.com. This list is no longer active.

From: Guy Harris <guy@xxxxxxxxxx>
Date: Wed, 12 Feb 2003 17:06:56 -0800
On Wed, Feb 12, 2003 at 04:52:32PM -0800, Guy Harris wrote:
> I checked for "tvb_remaining_length(tvb, llc_header_len)" instead -
> checking "tvb_offset_exists(tvb, llc_header_len + 1)" requires that
> there be at least *two* bytes starting at "llc_header_len" (the one at
> "llc_header_len" and the one at "llc_header_len + 1"), but one
> should be sufficient.

For that matter, why should that check be done at all?

If it's not done, and there's nothing in the tvbuff past the LLC header,
but the entire LLC header is in the tvbuff, "tvb_new_subset(tvb,
llc_header_len, -1, -1)" won't throw an exception, but it'll return a
tvbuff that, if any attempt is made to fetch data from it, will cause an
exception to be thrown.

If, for example, the reason why there's nothing past the LLC header is
that the snapshot length happened to cut the packet off right past the
LLC header, then the exception that will be thrown will probably be a
BoundsError exception, causing the packet to be reported as a Short
Frame...

....which is exactly what you'd *want* to happen, so that the user knows
that there was data that *could* have been shown to them if they'd
supplied a larger snapshot length.

Perhaps the check should see whether "tvb_reported_length_remaining(tvb,
llc_header_len)" is > 0, so that we skip the subdissectors only if there
really wasn't any data left in the packet - but even there, one could
perhaps argue that if the frame put on the wire was too short for the
payload that was supposed to be there, a ReportedBoundsError exception
should be thrown, so that it's reported as a Malformed Packet.