Wireshark · Wireshark-dev: Re: [Wireshark-dev] tvb_get_nstringz0

[John Thacker <johnthacker@xxxxxxxxx>]

On Fri, Mar 26, 2021 at 4:22 AM Dario Lombardo <lomato@xxxxxxxxx> wrote:

Hi,

I am a bit puzzled by the use of tvb_get_nstringz0. Let's say I have a packet 100 bytes long, that does NOT contain NUL. I call tvb_get_nstringz0 with a buffer 10 bytes long.

For what I can see, the function will seek the packet for NUL, stopping at the end of the packet, copying the result into the buffer. But the buffer is too short, resulting in a crash.

What's the error here? How is the caller sure their call won't be invalid? Should they always pass a long-enough buffer? Was the call to this function wrong in the first place? 

Hmm, looks okay to me.

When you call tvb_get_nstringz0() you pass in bufsize = 10.

tvb_get_nstringz0() calls _tvb_get_nstringz()

check_offset_len() runs to the end of the packet, setting len to 100.

Since len >= bufsize, it sets limit = bufsize.

stringlen = tvb_strnlen(tvb, abs_offset, limit - 1) looks at the first 9 bytes, doesn't find a NUL, returns -1

stringlen is -1, tvb_memcpy copies over limit (10) bytes into buffer from tvb, bytes_copies is set to 10, _tvb_get_nstringz() returns -1.

Since -1 was returned, tvb_get_nstringz0() sets buffer[9] to NULL, and returns 9.

The caller assures that the call won't be invalid by passing in the size of the buffer.

John Thacker