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

wslua: Fix memleak of unregistered ProtoField strings 

If a ProtoField object was created, but not linked to a Proto, then the
strings field and all elements (depending on type) would leak.

This is a follow-up to g79fef2ae and fixes the real issue in g44870fb1.

Change-Id: I01880a92bb20fae45f68c754b07daeb07630deec
Reviewed-on: https://code.wireshark.org/review/34872
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Vasil Velichkov <vvvelichkov@gmail.com>
Reviewed-by: Roland Knall <rknall@gmail.com>
This commit is contained in:
Stig Bjørlykke 2019-10-28 09:52:12 +01:00 committed by Roland Knall
parent 5fb897077e
commit 551745998e
6 changed files with 103 additions and 85 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
debian/libwireshark0.symbols vendored
View File

@ -1116,6 +1116,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
proto_find_first_finfo@Base 2.3.0
proto_find_undecoded_data@Base 1.99.3
proto_free_deregistered_fields@Base 1.12.2
proto_free_field_strings@Base 3.1.1
proto_get_data_protocol@Base 1.9.1
proto_get_finfo_ptr_array@Base 1.9.1
proto_get_first_protocol@Base 1.9.1

172
epan/proto.c
View File

@ -7675,6 +7675,95 @@ proto_add_deregistered_data (void *data)
g_ptr_array_add(deregistered_data, data);
}
void proto_free_field_strings (ftenum_t field_type, unsigned int field_display, const void *field_strings)
{
if (field_strings == NULL) {
return;
}
switch (field_type) {
case FT_FRAMENUM:
/* This is just an integer represented as a pointer */
break;
case FT_PROTOCOL: {
protocol_t *protocol = (protocol_t *)field_strings;
g_free((gchar *)protocol->short_name);
break;
}
case FT_BOOLEAN: {
true_false_string *tf = (true_false_string *)field_strings;
g_free((gchar *)tf->true_string);
g_free((gchar *)tf->false_string);
break;
}
case FT_UINT40:
case FT_INT40:
case FT_UINT48:
case FT_INT48:
case FT_UINT56:
case FT_INT56:
case FT_UINT64:
case FT_INT64: {
/*
* XXX - if it's BASE_RANGE_STRING, or
* BASE_EXT_STRING, should we free it?
*/
if (field_display & BASE_UNIT_STRING) {
unit_name_string *unit = (unit_name_string *)field_strings;
g_free((gchar *)unit->singular);
g_free((gchar *)unit->plural);
} else {
val64_string *vs64 = (val64_string *)field_strings;
while (vs64->strptr) {
g_free((gchar *)vs64->strptr);
vs64++;
}
}
break;
}
case FT_CHAR:
case FT_UINT8:
case FT_INT8:
case FT_UINT16:
case FT_INT16:
case FT_UINT24:
case FT_INT24:
case FT_UINT32:
case FT_INT32:
case FT_FLOAT:
case FT_DOUBLE: {
/*
* XXX - if it's BASE_RANGE_STRING, or
* BASE_EXT_STRING, should we free it?
*/
if (field_display & BASE_UNIT_STRING) {
unit_name_string *unit = (unit_name_string *)field_strings;
g_free((gchar *)unit->singular);
g_free((gchar *)unit->plural);
} else if (field_display & BASE_RANGE_STRING) {
range_string *rs = (range_string *)field_strings;
while (rs->strptr) {
g_free((gchar *)rs->strptr);
rs++;
}
} else {
value_string *vs = (value_string *)field_strings;
while (vs->strptr) {
g_free((gchar *)vs->strptr);
vs++;
}
}
break;
default:
break;
}
}
if (field_type != FT_FRAMENUM) {
g_free((void *)field_strings);
}
}
static void
free_deregistered_field (gpointer data, gpointer user_data _U_)
{
@ -7685,88 +7774,7 @@ free_deregistered_field (gpointer data, gpointer user_data _U_)
g_free((char *)hfi->abbrev);
g_free((char *)hfi->blurb);
if (hfi->strings) {
switch (hfi->type) {
case FT_FRAMENUM:
/* This is just an integer represented as a pointer */
break;
case FT_PROTOCOL: {
protocol_t *protocol = (protocol_t *)hfi->strings;
g_free((gchar *)protocol->short_name);
break;
}
case FT_BOOLEAN: {
true_false_string *tf = (true_false_string *)hfi->strings;
g_free ((gchar *)tf->true_string);
g_free ((gchar *)tf->false_string);
break;
}
case FT_UINT40:
case FT_INT40:
case FT_UINT48:
case FT_INT48:
case FT_UINT56:
case FT_INT56:
case FT_UINT64:
case FT_INT64: {
/*
* XXX - if it's BASE_RANGE_STRING, or
* BASE_EXT_STRING, should we free it?
*/
if (hfi->display & BASE_UNIT_STRING) {
unit_name_string *unit = (unit_name_string*)hfi->strings;
g_free ((gchar *)unit->singular);
g_free ((gchar *)unit->plural);
} else {
val64_string *vs64 = (val64_string *)hfi->strings;
while (vs64->strptr) {
g_free((gchar *)vs64->strptr);
vs64++;
}
}
break;
}
case FT_CHAR:
case FT_UINT8:
case FT_INT8:
case FT_UINT16:
case FT_INT16:
case FT_UINT24:
case FT_INT24:
case FT_UINT32:
case FT_INT32:
case FT_FLOAT:
case FT_DOUBLE: {
/*
* XXX - if it's BASE_RANGE_STRING, or
* BASE_EXT_STRING, should we free it?
*/
if (hfi->display & BASE_UNIT_STRING) {
unit_name_string *unit = (unit_name_string*)hfi->strings;
g_free ((gchar *)unit->singular);
g_free ((gchar *)unit->plural);
} else if (hfi->display & BASE_RANGE_STRING) {
range_string *rs = (range_string *)hfi->strings;
while (rs->strptr) {
g_free((gchar *)rs->strptr);
rs++;
}
} else {
value_string *vs = (value_string *)hfi->strings;
while (vs->strptr) {
g_free((gchar *)vs->strptr);
vs++;
}
}
break;
default:
break;
}
}
if (hfi->type != FT_FRAMENUM) {
g_free((void *)hfi->strings);
}
}
proto_free_field_strings(hfi->type, hfi->display, hfi->strings);
if (hfi->parent == -1)
g_slice_free(header_field_info, hfi);

7
epan/proto.h
View File

@ -2380,6 +2380,13 @@ proto_deregister_field (const int parent, gint hf_id);
WS_DLL_PUBLIC void
proto_add_deregistered_data (void *data);
/** Free strings in a field.
@param field_type the field type (one of FT_ values)
@param field_display field display value (one of BASE_ values)
@param field_strings field strings */
WS_DLL_PUBLIC void
proto_free_field_strings (ftenum_t field_type, unsigned int field_display, const void *field_strings);
/** Free fields deregistered in proto_deregister_field(). */
WS_DLL_PUBLIC void
proto_free_deregistered_fields (void);

1
epan/wslua/wslua_proto.c
View File

@ -638,6 +638,7 @@ int wslua_deregister_protocols(lua_State* L) {
/* Memory ownership was previously transferred to epan in Proto_commit */
f->name = NULL;
f->abbrev = NULL;
f->vs = NULL;
f->blob = NULL;
f->hfid = -2; /* Deregister ProtoField, freed in ProtoField__gc */

3
epan/wslua/wslua_proto_field.c
View File

@ -1462,10 +1462,11 @@ static int ProtoField__gc(lua_State* L) {
return 0;
}
/* Note: name, abbrev and blob will be NULL after Proto deregistration. */
/* Note: name, abbrev, blob and vs will be NULL after Proto deregistration. */
g_free(f->name);
g_free(f->abbrev);
g_free(f->blob);
proto_free_field_strings(f->type, f->base, f->vs);
g_free(f);
return 0;

4
test/lua/protofield.lua
View File

@ -79,7 +79,7 @@ test("ProtoField-char", success)
success = pcall(ProtoField.new, "char base NONE without valuestring", "test.char1", ftypes.CHAR, nil, base.NONE)
test("ProtoField-char-without-valuestring", not success)
success, test_proto.fields.char2 = pcall(ProtoField.new, "char base NONE with valuestring", "test.char2", ftypes.CHAR, {1, "Value"}, base.NONE)
success = pcall(ProtoField.new, "char base NONE with valuestring", "test.char2", ftypes.CHAR, {1, "Value"}, base.NONE)
test("ProtoField-char-with-valuestring", success)
success = pcall(ProtoField.new, "char base DEC", "test.char3", ftypes.CHAR, nil, base.DEC)
@ -88,7 +88,7 @@ test("ProtoField-char-base-dec", not success)
success = pcall(ProtoField.new, "char base UNIT_STRING", "test.char4", ftypes.CHAR, {" m"}, base.UNIT_STRING)
test("ProtoField-char-unit-string", not success)
success, test_proto.fields.char5 = pcall(ProtoField.new, "char base RANGE_STRING", "test.char5", ftypes.CHAR, {{1, 2, "Value"}}, base.RANGE_STRING)
success = pcall(ProtoField.new, "char base RANGE_STRING", "test.char5", ftypes.CHAR, {{1, 2, "Value"}}, base.RANGE_STRING)
test("ProtoField-char-range-string", success)
-- Field name: empty, illegal, incompatible