Wireshark mailing list archives

Re: [Wireshark-commits] master fe195c0: Don't throw for offset at end of TVB with len -1.


From: Evan Huus <eapache () gmail com>
Date: Fri, 25 Apr 2014 18:52:50 -0400

On Fri, Apr 25, 2014 at 6:13 PM, Guy Harris <guy () alum mit edu> wrote:

On Apr 25, 2014, at 2:48 PM, Evan Huus <eapache () gmail com> wrote:

I think conceptually what we need is a way to say "this item isn't
associated with any bytes at all, so don't do any bounds checks etc".
Negative offsets are already taken in the general case; do we need to
special-define a length of -2 for this?

It occurred to me after I sent this that we already have a way to note
an item isn't associated with any bytes: that's exactly what is
already meant by a length of 0. As such, I'm not sure I agree with my
previous statement that "given a tvbuff with available length n,
adding an item at offset n (zero-indexed) should throw an exception...
even if the item is a variable-length type... with length 0". I'm
starting to think that a length of 0 should mean we ignore the offset
entirely; it is irrelevant.

I'd actually like to see lengths become *unsigned*; currently, we need to do some hackery to make sure that

        length = tvb_get_ntohl(tvb, offset);

                ...

        offset += 4;

                ...

        proto_tree_add_whatever(..., offset, length, ...);

to handle the case where the length has the uppermost bit set.

Yes. While -1 is convenient for to-end-of-the-tvb, the signed/unsigned
thing is also inconvenient in a lot of other ways.

Perhaps there needs to be a call or calls to add items that don't correspond to packet data; those calls might take 
neither an offset argument nor a length argument (nor even a tvbuff argument?), and would not do any bounds checking. 
 Obviously, they'd have to take an argument corresponding to the value of the item.

If we don't check 0-length items at all (and stop using -1 as a valid
length) then I think this whole issue just sort of magically goes
away. The 0-length items in packet-frame.c won't cause problems, and
the '-1'-length FT_PROTOCOL in packet-frame.c "becomes" a 0-length
item for 0-length TVBs by dint of tvb_captured_length() returning 0.
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe


Current thread: