Wireshark · Wireshark-dev: Re: [Wireshark-dev] pcapng, must opt_comment string be 0-terminated?

[Chris Maynard <Chris.Maynard@xxxxxxxxx>]

Martin Kaiser <lists@...> writes:

> It's not that critial we but know the correct length and could do
> 
> opt_comment = g_strndup(option_content, oh.option_length)
> 
> instead.
> 
> If you agree, I can open a bugzilla item with a patch and sample capture
> that has an unterminated comment.

To avoid the possibility of reading past the end of option_content in the case
when oh.option_length > sizeof(option_content), I think it would be safer to do
this instead:

wblock->data.packet.opt_comment = g_strndup(option_content,
MIN(oh.option_length, sizeof(option_content)))

Note that this can't happen today because if the option length is greater than
sizeof(option_content), then we never get there, but I think the implementation
would work better to read as much as will fit into the buffer as possible, in
this case the 1st 99 characters of the string (plus the NULL-terminator), rather
than skipping it altogether.  The comment next to option_content indicates, "XXX
- size might need to be increased, if we see longer options", so it's apparently
a somewhat arbitrary limit.  I think as pcapng sees more and more use, it's only
a matter of time before longer and longer strings are added (or desired to be
added).  Even the examples shown at the end of section 2.5 of the pcapng spec
illustrate some strings approaching that limit, and those strings do not seem
overly long to me:

"This packet is the beginning of all of our problems" / "Packets 17-23 showing a
bogus TCP retransmission, as reported in bugzilla entry 1486!" / "Captured at
the southern plant" / "I've checked again, now it's working ok" / ...