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 <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

diff --git a/epan/ftypes/ftype-protocol.c b/epan/ftypes/ftype-protocol.c
index fbb79b8c7521abc46207da85bd957f63680da82f..8cdd1d4bf76cc7302be34c452260d8c333a2ccfe 100644 (file)
--- a/epan/ftypes/ftype-protocol.c
+++ b/epan/ftypes/ftype-protocol.c
@@ -9,6 +9,7 @@
 #include "config.h"
 
 #include <ftypes-int.h>
+#include <epan/strutil.h>
 #include <epan/to_str-int.h>
 #include <string.h>
 
@@ -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);
 }