On Fri, 04 Jan 2002 00:19:57 Tim Potter wrote:
> I was thinking that a BoundsError is the best exception to throw. If
> the dissector decides it wants some random number of bytes with the high
> bit set then it's obviously a malformed frame and ethereal should display
> it as such.
>
> > However, if you're going to check the count in the dissector, don't
> > you want to complain if count < 0, since dcerpc-nt's idea of a -1
> > count is not the same as tvbuff's idea of what -1 means. What about
> > the situation where count == 0 ?
>
> You're right - it should really be a < 0 in that if statement instead of
> < -1. I'm not sure what happens with count == 0. (-: I suspect any
> dissector requesting this will then do a for (i = 0; i < 0; i++) on it
> and then throw it away.
>
I've checked in the change to tvbuff.c to throw a BoundsError if
a length is < -1, but as we figured out, you should still check for
count == -1 in your dissector. BTW, just looking at the first
fix in the diff file, the change to prs_uint8s, I see that you
update offset by adding 'count':
offset += count;
return offset;
I don't know how prs_uint8s() is used, but you might want to check
for count == 0 too, since the loop might be something like:
while (offset < total_length) {
offset = prs_uint8s(tvb, offset, pinfo, tree, count=0,
data, name);
}
and thus offset never gets incremented, causing the loop to keep on
looping. Basically, anytime you use data from the packet for a
calculation,
it needs to be checked for sanity. We've had previous cases of
infinite-loops in
dissectors because of not checking the "length" value from a field in
a packet.
--gilbert