osmith
/
wireshark
forked from osmocom/wireshark
1
0
Fork 0
Code Pull Requests Releases Wiki Activity

tvb_get_nstringz() needs to terminate a string with a NUL if the 

end of the tvbuff is reached before the maximum_length passed by the
caller is reached and before a terminating NUL is found. In this case,
tvb_get_nstringz() returns a -1, but if the string is not artificially
terminated with a NUL by tvb_get_nstringz(), the
caller has no idea where the string should end because 1) the
return value "-1" gives the impression that the string ends
at the end of the buffer but 2) the string does
not end at the end of the buffer, but somewhere in the middle, due
to the packet being shorter than expected.

tvb_get_nstringz() and tvb_get_nstringz0() were both modified.

The FT_STRINGZ case in proto_tree_add_item() is made simpler.

During regression testing, when investigating a regression that I later
corrected, I discovered that strings added through proto_tree_add_item
(FT_STRING, FT_STRINGZ, and FT_UINT_STRING) leaked memory due to double
allocation of the string. The proto_tree_add_string*() functions do
not leak memory, since they only copy the string once. The memory
leak was fixed by adding another argument to the static function
proto_tree_set_string() to let the string ftype code know to g_strdup()
the string or not.

svn path=/trunk/; revision=4891
This commit is contained in:
Gilbert Ramirez 2002-03-06 19:17:06 +00:00
parent 6ff05ff765
commit 6aad6e40b8
3 changed files with 86 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
epan/proto.c
View File

@ -1,7 +1,7 @@
/* proto.c
* Routines for protocol tree
*
* $Id: proto.c,v 1.56 2002/03/02 20:48:10 guy Exp $
* $Id: proto.c,v 1.57 2002/03/06 19:17:05 gram Exp $
*
* Ethereal - Network traffic analyzer
* By Gerald Combs <gerald@ethereal.com>
@ -94,7 +94,7 @@ proto_tree_set_bytes_tvb(field_info *fi, tvbuff_t *tvb, gint offset, gint length
static void
proto_tree_set_time(field_info *fi, nstime_t *value_ptr);
static void
proto_tree_set_string(field_info *fi, const char* value);
proto_tree_set_string(field_info *fi, const char* value, gboolean);
static void
proto_tree_set_string_tvb(field_info *fi, tvbuff_t *tvb, gint start, gint length);
static void
@ -609,19 +609,18 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
break;
case FT_STRINGZ:
/* In this case, length signifies maximum length. */
/* This g_strdup'ed memory is freed in proto_tree_free_node() */
string = g_malloc(length);
CLEANUP_PUSH(g_free, string);
found_length = tvb_get_nstringz(tvb, start, length, string);
if (found_length < 1) {
found_length = tvb_get_nstringz0(tvb, start, length, string);
}
found_length = tvb_get_nstringz0(tvb, start, length, string);
CLEANUP_POP;
proto_tree_set_string(new_fi, string);
proto_tree_set_string(new_fi, string, TRUE);
new_fi->length = found_length + 1;
break;
@ -643,12 +642,13 @@ proto_tree_add_item(proto_tree *tree, int hfindex, tvbuff_t *tvb,
break;
}
CLEANUP_POP;
/* Don't add to proto_item to proto_tree until now so that any exceptions
/* Don't add new node to proto_tree until now so that any exceptions
* raised by a tvbuff access method doesn't leave junk in the proto_tree. */
pi = proto_tree_add_node(tree, new_fi);
CLEANUP_POP;
/* If the proto_tree wants to keep a record of this finfo
* for quick lookup, then record it. */
hash = PTREE_DATA(tree)->interesting_hfids;
@ -1098,7 +1098,8 @@ proto_tree_set_uint64_tvb(field_info *fi, tvbuff_t *tvb, gint start, gboolean li
proto_tree_set_uint64(fi, tvb_get_ptr(tvb, start, 8), little_endian);
}
/* Add a FT_STRING to a proto_tree */
/* Add a FT_STRING to a proto_tree. Creates own copy of string,
* and frees it when the proto_tree is destroyed. */
proto_item *
proto_tree_add_string(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
gint length, const char* value)
@ -1114,7 +1115,7 @@ proto_tree_add_string(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint start,
g_assert(hfinfo->type == FT_STRING);
pi = proto_tree_add_pi(tree, hfindex, tvb, start, &length, &new_fi);
proto_tree_set_string(new_fi, value);
proto_tree_set_string(new_fi, value, FALSE);
return pi;
}
@ -1156,9 +1157,10 @@ proto_tree_add_string_format(proto_tree *tree, int hfindex, tvbuff_t *tvb, gint
/* Set the FT_STRING value */
static void
proto_tree_set_string(field_info *fi, const char* value)
proto_tree_set_string(field_info *fi, const char* value,
gboolean already_allocated)
{
fvalue_set(fi->value, (gpointer) value, FALSE);
fvalue_set(fi->value, (gpointer) value, already_allocated);
}
static void
@ -1174,7 +1176,7 @@ proto_tree_set_string_tvb(field_info *fi, tvbuff_t *tvb, gint start, gint length
string = g_malloc(length + 1);
tvb_memcpy(tvb, string, start, length);
string[length] = '\0';
fvalue_set(fi->value, string, TRUE);
proto_tree_set_string(fi, string, TRUE);
}
/* Add a FT_ETHER to a proto_tree */

75
epan/tvbuff.c
View File

@ -9,7 +9,7 @@
* the data of a backing tvbuff, or can be a composite of
* other tvbuffs.
*
* $Id: tvbuff.c,v 1.30 2002/02/18 01:08:42 guy Exp $
* $Id: tvbuff.c,v 1.31 2002/03/06 19:17:05 gram Exp $
*
* Copyright (c) 2000 by Gilbert Ramirez <gram@alumni.rice.edu>
*
@ -1324,13 +1324,25 @@ tvb_format_text(tvbuff_t *tvb, gint offset, gint size)
* Returns length of string (not including terminating NUL), or -1 if the string was
* truncated in the buffer due to not having reached the terminating NUL.
* In this way, it acts like snprintf().
*
* When processing a packet where the remaining number of bytes is less
* than maxlength, an exception is not thrown if the end of the packet
* is reached before the NUL is found. If no NUL is found before reaching
* the end of the short packet, -1 is still returned, and the string
* is truncated with a NUL, albeit not at buffer[maxlength], but
* at the correct spot, terminating the string.
*
* *bytes_copied will contain the number of bytes actually copied,
* including the terminating-NUL.
*/
gint
tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer)
_tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer,
gint *bytes_copied)
{
gint stringlen;
guint abs_offset, junk_length;
gint limit, len;
gboolean decreased_max = FALSE;
check_offset_length(tvb, offset, 0, &abs_offset, &junk_length);
@ -1339,34 +1351,71 @@ tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer)
return 0;
}
/* Only copy to end of tvbuff, w/o throwing exception. */
/* Only read to end of tvbuff, w/o throwing exception. */
len = tvb_length_remaining(tvb, abs_offset);
/* This should not happen because check_offset_length() would
* have already thrown an exception if 'offset' were out-of-bounds.
*/
g_assert(len != -1);
/* check_offset_length() won't throw an exception if we're
* looking at the byte immediately after the end of the tvbuff. */
if (len == 0) {
THROW(ReportedBoundsError);
}
if ((guint)len < maxlength) {
limit = maxlength - (tvb_length(tvb) - abs_offset);
/* This should not happen because check_offset_length() would
* have already thrown an exception if 'offset' were out-of-bounds.
*/
g_assert(len != -1);
if ((guint)len < maxlength) {
limit = len;
decreased_max = TRUE;
}
else {
limit = maxlength;
}
stringlen = tvb_strnlen(tvb, abs_offset, limit);
/* If NUL wasn't found, copy the data and return -1 */
if (stringlen == -1) {
tvb_memcpy(tvb, buffer, abs_offset, limit);
if (decreased_max) {
buffer[limit] = 0;
/* Add 1 for the extra NUL that we set at buffer[limit],
* pretending that it was copied as part of the string. */
*bytes_copied = limit + 1;
}
else {
*bytes_copied = limit;
}
return -1;
}
/* Copy the string to buffer */
tvb_memcpy(tvb, buffer, abs_offset, stringlen + 1);
*bytes_copied = stringlen + 1;
return stringlen;
}
/* Looks for a stringz (NUL-terminated string) in tvbuff and copies
* no more than maxlength number of bytes, including terminating NUL, to buffer.
* Returns length of string (not including terminating NUL), or -1 if the string was
* truncated in the buffer due to not having reached the terminating NUL.
* In this way, it acts like snprintf().
*
* When processing a packet where the remaining number of bytes is less
* than maxlength, an exception is not thrown if the end of the packet
* is reached before the NUL is found. If no NUL is found before reaching
* the end of the short packet, -1 is still returned, and the string
* is truncated with a NUL, albeit not at buffer[maxlength], but
* at the correct spot, terminating the string.
*/
gint
tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer)
{
gint bytes_copied;
return _tvb_get_nstringz(tvb, offset, maxlength, buffer, &bytes_copied);
}
/* Like tvb_get_nstringz(), but never returns -1. The string is guaranteed to
* have a terminating NUL. If the string was truncated when copied into buffer,
* a NUL is placed at the end of buffer to terminate it.
@ -1374,13 +1423,13 @@ tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer)
gint
tvb_get_nstringz0(tvbuff_t *tvb, gint offset, guint maxlength, guint8* buffer)
{
gint len;
gint len, bytes_copied;
len = tvb_get_nstringz(tvb, offset, maxlength, buffer);
len = _tvb_get_nstringz(tvb, offset, maxlength, buffer, &bytes_copied);
if (len == -1) {
buffer[maxlength] = 0;
return maxlength - 1;
return bytes_copied - 1;
}
else {
return len;

9
epan/tvbuff.h
View File

@ -9,7 +9,7 @@
* the data of a backing tvbuff, or can be a composite of
* other tvbuffs.
*
* $Id: tvbuff.h,v 1.22 2002/02/18 01:08:42 guy Exp $
* $Id: tvbuff.h,v 1.23 2002/03/06 19:17:06 gram Exp $
*
* Copyright (c) 2000 by Gilbert Ramirez <gram@alumni.rice.edu>
*
@ -308,6 +308,13 @@ extern guint8 * tvb_format_text(tvbuff_t *tvb, gint offset, gint size);
* Returns length of string (not including terminating NUL), or -1 if the string was
* truncated in the buffer due to not having reached the terminating NUL.
* In this way, it acts like snprintf().
*
* When processing a packet where the remaining number of bytes is less
* than maxlength, an exception is not thrown if the end of the packet
* is reached before the NUL is found. If no NUL is found before reaching
* the end of the short packet, -1 is still returned, and the string
* is truncated with a NUL, albeit not at buffer[maxlength], but
* at the correct spot, terminating the string.
*/
extern gint tvb_get_nstringz(tvbuff_t *tvb, gint offset, guint maxlength,
guint8* buffer);