From 69e2603c48d04a675785d9e7bad162ebb9a83b07 Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Tue, 24 Apr 2018 19:29:35 +0200 Subject: [PATCH] ftypes: fix memleak when converting protocol values When converting byte array strings to a FT_PROTOCOL value (for example, when using a display filter such as `eth contains aa:bb`), the converted memory in GByteArray was not freed. If an error occurred (the value cannot be parsed as hex string), then an error message was leaked. Fix the above issues and avoid an unnecessary g_memdup. Change-Id: I3a076b3a2384b1a0e15ea8518f2e0f66a7b6ea49 Reviewed-on: https://code.wireshark.org/review/27130 Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- epan/ftypes/ftype-protocol.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/epan/ftypes/ftype-protocol.c b/epan/ftypes/ftype-protocol.c index fbb79b8c75..8cdd1d4bf7 100644 --- a/epan/ftypes/ftype-protocol.c +++ b/epan/ftypes/ftype-protocol.c @@ -9,6 +9,7 @@ #include "config.h" #include +#include #include #include @@ -41,16 +42,11 @@ value_set(fvalue_t *fv, tvbuff_t *value, const gchar *name) /* Free up the old value, if we have one */ value_free(fv); + /* Set the protocol description and an (optional, nullable) tvbuff. */ fv->value.protocol.tvb = value; fv->value.protocol.proto_string = g_strdup(name); } -static void -free_tvb_data(void *data) -{ - g_free(data); -} - static gboolean val_from_string(fvalue_t *fv, const char *s, gchar **err_msg _U_) { @@ -67,37 +63,37 @@ val_from_string(fvalue_t *fv, const char *s, gchar **err_msg _U_) (guint)strlen(s), (gint)strlen(s)); /* Let the tvbuff know how to delete the data. */ - tvb_set_free_cb(new_tvb, free_tvb_data); + tvb_set_free_cb(new_tvb, g_free); /* And let us know that we need to free the tvbuff */ fv->tvb_is_private = TRUE; + /* This "field" is a value, it has no protocol description. */ fv->value.protocol.tvb = new_tvb; - fv->value.protocol.proto_string = g_strdup(s); + fv->value.protocol.proto_string = NULL; return TRUE; } static gboolean val_from_unparsed(fvalue_t *fv, const char *s, gboolean allow_partial_value _U_, gchar **err_msg) { - fvalue_t *fv_bytes; tvbuff_t *new_tvb; - guint8 *private_data; /* Free up the old value, if we have one */ value_free(fv); + fv->value.protocol.tvb = NULL; + fv->value.protocol.proto_string = NULL; /* Does this look like a byte string? */ - fv_bytes = fvalue_from_unparsed(FT_BYTES, s, TRUE, NULL); - if (fv_bytes) { + GByteArray *bytes = g_byte_array_new(); + if (hex_str_to_bytes(s, bytes, TRUE)) { /* Make a tvbuff from the bytes */ - private_data = (guint8 *)g_memdup(fv_bytes->value.bytes->data, - fv_bytes->value.bytes->len); - new_tvb = tvb_new_real_data(private_data, - fv_bytes->value.bytes->len, - fv_bytes->value.bytes->len); + new_tvb = tvb_new_real_data(bytes->data, bytes->len, bytes->len); /* Let the tvbuff know how to delete the data. */ - tvb_set_free_cb(new_tvb, free_tvb_data); + tvb_set_free_cb(new_tvb, g_free); + + /* Free GByteArray, but keep data. */ + g_byte_array_free(bytes, FALSE); /* And let us know that we need to free the tvbuff */ fv->tvb_is_private = TRUE; @@ -105,6 +101,9 @@ val_from_unparsed(fvalue_t *fv, const char *s, gboolean allow_partial_value _U_, return TRUE; } + /* Not a byte array, forget about it. */ + g_byte_array_free(bytes, TRUE); + /* Treat it as a string. */ return val_from_string(fv, s, err_msg); }