Re: [Ethereal-dev] What does the "maxlength" argument to "tvb_get_nstringz*" mean?

[Guy Harris <gharris@xxxxxxxxx>]

On Tue, Apr 29, 2003 at 10:05:42AM -0500, Gerald Combs wrote:
> On Mon, 28 Apr 2003, Guy Harris wrote:
> 
> > The question is whether the "maxlength" argument to those routines
> > should be
> > 
> > 	1) the maximum number of non-'\0' characters copied
> > 
> > or
> > 
> > 	2) the size of the buffer into which the string is being copied.

	...

> I'm leaning towards 2), since it errs on the side of safety.  If 1) is
> implemented the caller assumes 2), you have a potential buffer overflow.  
> Conversely, if 2) is implemented and 1) is assumed you waste a byte of
> memory.  According to Timo's original message, about half the dissectors
> that call tvb_get_nstringz*() assume 2).  Here's his list:
> 
> packet-smb-browse.c:676
> packet-rsync.c:219
> packet-quake.c:164,186,221,258,285,293,301,349,386,410,418
> packet-quake2.c:120,354,375
> packet-quake3.c:178
> packet-pptp.c:2123
> packet-ospf.c:559
> packet-giop.c:2888
> packet-aim.c:732
> packet-smpp.c:622,667
> packet-tsp.c:176:
> 
> It looks like gryphon/packet-gryphon.c:1547 should be added to the list.

Actually, the code at packet-ospf.c:559 was assuming it's 1) - the field
is 64 bits, or 8 bytes, so it needed to be extracted into an 8+1-byte
buffer to leave room for the trailing '\0', and the argument to
"tvb_get_nstringz0()" should be 8+1.

I've checked in a change to do that; the other calls should perhaps be
checked as well.