Ethereal-dev: Re: [Ethereal-dev] What does the "maxlength" argument to "tvb_get_nstringz*" mea
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.
