Ethereal-dev: [Ethereal-dev] What does the "maxlength" argument to "tvb_get_nstringz*" mean?
On Sun, Apr 27, 2003 at 11:03:27PM -0500, Gerald Combs wrote:
> gerald 2003/04/27 23:03:27 CDT
>
> Modified files:
> epan tvbuff.c
> Log:
> Fix several buffer and integer overflow issues discovered by Timo Sirainen.
>
> tvbuff.c:
>
> Lots of existing code assumes that you can safely do the following:
>
> #define MAX_BUF 64
> guint8 *buf[MAX_BUF];
> ...
>
> tvb_get_nstringz0 (tvb, offset, MAX_BUF, buf, &bytes_copied);
>
> In reality, tvb_get_nstringz*() can potentially write one byte past
> "buf". Modify _tvb_get_nstringz() not to do that.
Hmm.
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.
At least some code appears to assume it's 1), e.g. the FTP, iSCSI and
MGCP dissectors; some other code appears to assume it's 2), e.g., I
suspect, the AIM dissector (although I'm not sure of that - it depends
on what the buddy name length is; the buffer is MAX_BUDDYNAME_LENGTH
bytes long).
If it's supposed to be 1), the change should be backed out and all the
code that assumes it's 2) needs to be fixed (typically by increasing the
size of the buffer by 1, to leave room for the trailing '\0').
If it's supposed to be 2), "tvb_get_nstringz0()" needs to be changed to
set "buffer[maxlength-1]" to '\0' rather than setting
"buffer[maxlength]" to '\0', and the code that assumes it's 1) needs to
be fixed (by passing the size of the buffer, rather than the size of the
buffer - 1, as the maximum length argument).
