airpdcap: add free_key_string function, fix memleaks

Do not leak the key and SSID. Note that there are still some leaks in
the GTK UI related to get_wireshark_keys(), but I did not track them
down.

Caught by LeakSanitizer.

Change-Id: I639166e6ea457605d6ae0ebd58e56d7594a7b7db
Reviewed-on: https://code.wireshark.org/review/10860
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Michael Mann <mmann78@netscape.net>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>

5 files changed, 28 insertions, 10 deletions

diff --git a/debian/libwireshark0.symbols b/debian/libwireshark0.symbols
index 5340319670..5f9177f608 100644
--- a/debian/libwireshark0.symbols
+++ b/debian/libwireshark0.symbols
@@ -574,6 +574,7 @@ libwireshark.so.0 libwireshark0 #MINVER#
  frame_data_set_after_dissect@Base 1.9.1
  frame_data_set_before_dissect@Base 1.9.1
  free_frame_data_sequence@Base 1.12.0~rc1
+ free_key_string@Base 2.0.0~rc1
  free_rtd_table@Base 1.99.8
  free_srt_table@Base 1.99.8
  free_srt_table_data@Base 1.99.8
diff --git a/epan/crypt/airpdcap.c b/epan/crypt/airpdcap.c
index 14ea621ba1..a5cc0fdf4e 100644
--- a/epan/crypt/airpdcap.c
+++ b/epan/crypt/airpdcap.c
@@ -1968,6 +1968,16 @@ parse_key_string(gchar* input_string, guint8 key_type)
     return NULL;
 }
 
+void
+free_key_string(decryption_key_t *dk)
+{
+    if (dk->key)
+        g_string_free(dk->key, TRUE);
+    if (dk->ssid)
+        g_byte_array_free(dk->ssid, TRUE);
+    g_free(dk);
+}
+
 /*
  * Returns a newly allocated string representing the given decryption_key_t
  * struct, or NULL if something is wrong...
diff --git a/epan/crypt/airpdcap_user.h b/epan/crypt/airpdcap_user.h
index d2ac85dd9b..aeca3f08c4 100644
--- a/epan/crypt/airpdcap_user.h
+++ b/epan/crypt/airpdcap_user.h
@@ -211,7 +211,7 @@ typedef struct _AIRPDCAP_KEYS_COLLECTION {
  * - AIRPDCAP_KEY_TYPE_WPA_PSK (WPA + 256-bit raw key)
  * @return A pointer to a freshly-g_malloc()ed decryption_key_t struct on
  *   success, or NULL on failure.
- * @see get_key_string()
+ * @see get_key_string(), free_key_string()
  */
 WS_DLL_PUBLIC
 decryption_key_t*
@@ -228,6 +228,15 @@ WS_DLL_PUBLIC
 gchar*
 get_key_string(decryption_key_t* dk);
 
+/**
+ * Releases memory associated with a given decryption_key_t struct.
+ * @param dk [IN] Pointer to the key to be freed
+ * @see parse_key_string()
+ */
+WS_DLL_PUBLIC
+void
+free_key_string(decryption_key_t *dk);
+
 /******************************************************************************/
 
 #endif /* _AIRPDCAP_USER_H */
diff --git a/epan/dissectors/packet-ieee80211.c b/epan/dissectors/packet-ieee80211.c
index 744824e9ab..75dc83fb68 100644
--- a/epan/dissectors/packet-ieee80211.c
+++ b/epan/dissectors/packet-ieee80211.c
@@ -193,7 +193,7 @@ uat_wep_key_record_update_cb(void* r, char** err)
 
   if (dk != NULL) {
     dk_type = dk->type;
-    g_free(dk);
+    free_key_string(dk);
     switch (dk_type) {
       case AIRPDCAP_KEY_TYPE_WEP:
       case AIRPDCAP_KEY_TYPE_WEP_40:
@@ -18781,14 +18781,16 @@ set_airpdcap_keys(void)
           keys.nKeys += 1;
         }
       }
-      g_free(dk);
+      free_key_string(dk);
+      if (bytes) {
+        g_byte_array_free(bytes, TRUE);
+        bytes = NULL;
+      }
     }
   }
 
   /* Now set the keys */
   AirPDcapSetKeys(&airpdcap_ctx, keys.Keys, keys.nKeys);
-  if (bytes)
-    g_byte_array_free(bytes, TRUE);
 
 }
 
diff --git a/ui/gtk/airpcap_gui_utils.c b/ui/gtk/airpcap_gui_utils.c
index 50a83eb741..c4c3f18e95 100644
--- a/ui/gtk/airpcap_gui_utils.c
+++ b/ui/gtk/airpcap_gui_utils.c
@@ -544,7 +544,6 @@ static gboolean
 load_wlan_driver_wep_keys(void)
 {
     keys_cb_data_t* user_data;
-    guint i;
 
     /* Retrieve the wlan preferences */
     wlan_prefs = prefs_find_module("wlan");
@@ -574,10 +573,7 @@ load_wlan_driver_wep_keys(void)
 
     /* FREE MEMORY */
     /* free the WEP key string */
-    for(i=0;i<g_list_length(user_data->list);i++)
-    {
-        g_free(g_list_nth(user_data->list,i)->data);
-    }
+    g_list_foreach(user_data->list, (GFunc)free_key_string, NULL);
 
     /* free the (empty) list */
     g_list_free(user_data->list);