wslua_field: fix memory leaks in Field_new

Change the "Field" type to actually point to a structure. Do not cheat
and overload the pointer to mean "char*" in one context, and
"header_field_info*" in another. It was very confusing.

Implement Field__gc to free the Field structure that was allocated in
Field_new. This fixes the memory leak in Field_new.

Now the test_wslua_field test passes when executed with ASAN and a bunch
of other wslua tests also improve.

Change-Id: Ibc4318b76bb893151fd40c3fbc595402fba7a60a
Reviewed-on: https://code.wireshark.org/review/31743
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>

2 files changed, 47 insertions, 40 deletions

diff --git a/epan/wslua/wslua.h b/epan/wslua/wslua.h
index eb6229fd6e..92c7a9318a 100644
--- a/epan/wslua/wslua.h
+++ b/epan/wslua/wslua.h
@@ -217,6 +217,12 @@ struct _wslua_treeitem {
     gboolean expired;
 };
 
+// Internal structure for wslua_field.c to track info about registered fields.
+struct _wslua_header_field_info {
+    char *name;
+    header_field_info *hfi;
+};
+
 struct _wslua_field_info {
     field_info *ws_fi;
     gboolean expired;
@@ -320,7 +326,7 @@ typedef address* Address;
 typedef nstime_t* NSTime;
 typedef gint64 Int64;
 typedef guint64 UInt64;
-typedef header_field_info** Field;
+typedef struct _wslua_header_field_info* Field;
 typedef struct _wslua_field_info* FieldInfo;
 typedef struct _wslua_tap* Listener;
 typedef struct _wslua_tw* TextWindow;
diff --git a/epan/wslua/wslua_field.c b/epan/wslua/wslua_field.c
index 5ae3e12661..266eea939b 100644
--- a/epan/wslua/wslua_field.c
+++ b/epan/wslua/wslua_field.c
@@ -515,6 +515,7 @@ WSLUA_CLASS_DEFINE(Field,FAIL_ON_NULL("Field"));
    Once created, it is used *inside* the callback functions, to generate a `FieldInfo` object.
  */
 
+/* Array of Field (struct _wslua_header_field_info*) pointers.*/
 static GPtrArray* wanted_fields = NULL;
 static dfilter_t* wslua_dfilter = NULL;
 
@@ -555,20 +556,14 @@ void lua_prime_all_fields(proto_tree* tree _U_) {
 
     for(i=0; i < wanted_fields->len; i++) {
         Field f = (Field)g_ptr_array_index(wanted_fields,i);
-        gchar* name = *((gchar**)f);
 
-        *f = proto_registrar_get_byname(name);
-
-        if (!*f) {
-            report_failure("Could not find field `%s'",name);
-            *f = NULL;
-            g_free(name);
+        f->hfi = proto_registrar_get_byname(f->name);
+        if (!f->hfi) {
+            report_failure("Could not find field `%s'", f->name);
             continue;
         }
 
-        g_free(name);
-
-        g_string_append_printf(fake_tap_filter," || %s",(*f)->abbrev);
+        g_string_append_printf(fake_tap_filter, " || %s", f->hfi->abbrev);
         fake_tap = TRUE;
     }
 
@@ -612,10 +607,10 @@ WSLUA_CONSTRUCTOR Field_new(lua_State *L) {
         return 0;
     }
 
-    f = (Field)g_malloc(sizeof(void*));
-    *f = (header_field_info*)(void*)g_strdup(name); /* cheating */
+    f = (Field)g_new0(struct _wslua_header_field_info, 1);
+    f->name = g_strdup(name);
 
-    g_ptr_array_add(wanted_fields,f);
+    g_ptr_array_add(wanted_fields, f);
 
     pushField(L,f);
     WSLUA_RETURN(1); /* The field extractor */
@@ -653,27 +648,22 @@ WSLUA_CONSTRUCTOR Field_list(lua_State *L) {
     WSLUA_RETURN(1); /* The array table of field filter names */
 }
 
-/* the following is used in Field_get_xxx functions later */
+/* the following is used in Field_get_xxx functions later. If called early
+ * (wanted_fields is not NULL), it will try to retrieve information directly.
+ * Otherwise it uses a cached field that was loaded in lua_prime_all_fields. */
 #define GET_HFINFO_MEMBER(luafunc, member)                          \
     if (wanted_fields) {                                            \
-        /* before registration, so it's a gchar** of the abbrev */  \
-        const gchar* name = (const gchar*) *fi;                     \
-        if (name) {                                                 \
-            hfinfo = proto_registrar_get_byname(name);              \
-            if (!hfinfo) {                                          \
-                /* could be a Lua-created field */                  \
-                ProtoField pf = wslua_is_field_available(L, name);  \
-                if (pf) {                                           \
-                    luafunc(L, pf->member);                         \
-                    return 1;                                       \
-                }                                                   \
+        hfinfo = proto_registrar_get_byname(f->name);               \
+        if (!hfinfo) {                                              \
+            /* could be a Lua-created field */                      \
+            ProtoField pf = wslua_is_field_available(L, f->name);   \
+            if (pf) {                                               \
+                luafunc(L, pf->member);                             \
+                return 1;                                           \
             }                                                       \
-        } else {                                                    \
-            luaL_error(L, "Field." #member ": unknown field");      \
-            return 0;                                               \
         }                                                           \
     } else {                                                        \
-        hfinfo = *fi;                                               \
+        hfinfo = f->hfi;                                            \
     }                                                               \
                                                                     \
     if (hfinfo) {                                                   \
@@ -687,7 +677,7 @@ WSLUA_CONSTRUCTOR Field_list(lua_State *L) {
    @since 1.99.8
  */
 static int Field_get_name(lua_State* L) {
-    Field fi = checkField(L,1);
+    Field f = checkField(L,1);
     header_field_info* hfinfo = NULL;
 
     GET_HFINFO_MEMBER(lua_pushstring, abbrev);
@@ -700,7 +690,7 @@ static int Field_get_name(lua_State* L) {
    @since 1.99.8
  */
 static int Field_get_display(lua_State* L) {
-    Field fi = checkField(L,1);
+    Field f = checkField(L,1);
     header_field_info* hfinfo = NULL;
 
     GET_HFINFO_MEMBER(lua_pushstring, name);
@@ -713,7 +703,7 @@ static int Field_get_display(lua_State* L) {
    @since 1.99.8
  */
 static int Field_get_type(lua_State* L) {
-    Field fi = checkField(L,1);
+    Field f = checkField(L,1);
     header_field_info* hfinfo = NULL;
 
     GET_HFINFO_MEMBER(lua_pushnumber, type);
@@ -724,7 +714,7 @@ static int Field_get_type(lua_State* L) {
 WSLUA_METAMETHOD Field__call (lua_State* L) {
     /* Obtain all values (see `FieldInfo`) for this field. */
     Field f = checkField(L,1);
-    header_field_info* in = *f;
+    header_field_info* in = f->hfi;
     int items_found = 0;
 
     if (! in) {
@@ -756,17 +746,28 @@ WSLUA_METAMETHOD Field__tostring(lua_State* L) {
     /* Obtain a string with the field filter name. */
     Field f = checkField(L,1);
 
-    if (wanted_fields) {
-        lua_pushstring(L,*((gchar**)f));
+    if (f->hfi) {
+        /* If a field was found, return the actual field info. */
+        lua_pushstring(L, f->hfi->abbrev);
     } else {
-        lua_pushstring(L,(*f)->abbrev);
+        lua_pushstring(L, f->name);
     }
 
     return 1;
 }
 
-static int Field__gc(lua_State* L _U_) {
-    /* do NOT free Field */
+static int Field__gc(lua_State* L) {
+    Field f = toField(L,1);
+    if (!f) return 0;
+
+    // If out of scope before lua_prime_all_fields is even called, be sure to
+    // remove the pointer to avoid a use-after-free.
+    if (wanted_fields) {
+        g_ptr_array_remove_fast(wanted_fields, f);
+    }
+
+    g_free(f->name);
+    g_free(f);
     return 0;
 }
 
@@ -814,7 +815,7 @@ int wslua_deregister_fields(lua_State* L _U_) {
 }
 
 /*
- * Editor modelines  -  http://www.wireshark.org/tools/modelines.html
+ * Editor modelines  -  https://www.wireshark.org/tools/modelines.html
  *
  * Local variables:
  * c-basic-offset: 4