[PATCH 1/4] wifi: Fix use-after-free when tethering is disabled.

The bug frequently causes junk being written to the
Tethering.Identifier and Tethering.Passphrase entries in the settings
file. This in turn prevents Connman from starting up at all the next
time it is run.

To reproduce with connmanctl, use the following commands:

    enable wifi
    tether wifi on MyNetwork ThePassphrase
    tether wifi off

Then have a look at the settings file.

Disabling tethering causes a call of remove_ssid() from
interface_select_network_result() in gsupplicant/supplicant.c, which
frees the SSID and passphrase strings of the given GSupplicantSSID
structure (originally initialized by the wifi plugin). These strings,
however, are shared with a connman_technology structure, and they are
accessed in technology_save() (called indirectly from set_property()
in technology.c) after they have been freed.

To fix this bug, the affected strings are copied into the
GSupplicantSSID structure instead of simply assigning them. We make
sure (1) to free them in case supplicant didn't take the
GSupplicantSSID structure due to an error, and (2) to avoid double
frees in ap_create_callback() and sta_remove_callback().

Originally, there were two GSupplicantSSID instances in the wifi
plugin: one for the supplicant, and one stored in the wifi_data's
tethering_param structure. The latter, however, was never used, so
this commit removes its allocation to avoid a new memory leak, to save
memory, and to generally avoid confusion.

The bug fixed by this commit was introduced in 481d08f108.
---
 plugins/wifi.c | 54 ++++++++++++++++++++++----------------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/plugins/wifi.c b/plugins/wifi.c
index ed7437f5..ba5aa2d7 100644
--- a/plugins/wifi.c
+++ b/plugins/wifi.c
@@ -2547,6 +2547,16 @@ static bool handle_sae_authentication_failure(struct connman_network *network,
 	return true;
 }

+static void wifi_data_free_tethering_info(struct wifi_data *wifi)
+{
+	if (!wifi->tethering_param)
+		return;
+
+	g_assert(wifi->tethering_param->ssid == NULL);
+	g_free(wifi->tethering_param);
+	wifi->tethering_param = NULL;
+}
+
 static void interface_state(GSupplicantInterface *interface)
 {
 	struct connman_network *network;
@@ -2568,11 +2578,7 @@ static void interface_state(GSupplicantInterface *interface)
 		return;

 	if (state == G_SUPPLICANT_STATE_COMPLETED) {
-		if (wifi->tethering_param) {
-			g_free(wifi->tethering_param->ssid);
-			g_free(wifi->tethering_param);
-			wifi->tethering_param = NULL;
-		}
+		wifi_data_free_tethering_info(wifi);

 		if (wifi->tethering)
 			stop_autoscan(device);
@@ -2832,9 +2838,7 @@ static void ap_create_fail(GSupplicantInterface *interface)
 			connman_technology_tethering_notify(wifi_technology,false);
 		}

-		g_free(wifi->tethering_param->ssid);
-		g_free(wifi->tethering_param);
-		wifi->tethering_param = NULL;
+		wifi_data_free_tethering_info(wifi);
 	}
 }

@@ -3387,7 +3391,7 @@ static GSupplicantSSID *ssid_ap_init(const struct connman_technology *technology
 		return NULL;

 	ap->mode = G_SUPPLICANT_MODE_MASTER;
-	ap->ssid = ssid;
+	ap->ssid = g_strdup(ssid);
 	ap->ssid_len = strlen(ssid);
 	ap->scan_ssid = 0;
 	if (freq)
@@ -3403,7 +3407,7 @@ static GSupplicantSSID *ssid_ap_init(const struct connman_technology *technology
 	       ap->protocol = G_SUPPLICANT_PROTO_RSN;
 	       ap->pairwise_cipher = G_SUPPLICANT_PAIRWISE_CCMP;
 	       ap->group_cipher = G_SUPPLICANT_GROUP_CCMP;
-	       ap->passphrase = passphrase;
+	       ap->passphrase = g_strdup(passphrase);
 	}

 	return ap;
@@ -3423,9 +3427,7 @@ static void ap_start_callback(int result, GSupplicantInterface *interface,

 		if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
 			connman_technology_tethering_notify(info->technology, false);
-			g_free(info->wifi->tethering_param->ssid);
-			g_free(info->wifi->tethering_param);
-			info->wifi->tethering_param = NULL;
+			wifi_data_free_tethering_info(info->wifi);
 		}
 	}

@@ -3448,14 +3450,10 @@ static void ap_create_callback(int result,

 		if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
 			connman_technology_tethering_notify(info->technology, false);
-			g_free(info->wifi->tethering_param->ssid);
-			g_free(info->wifi->tethering_param);
-			info->wifi->tethering_param = NULL;
-
+			wifi_data_free_tethering_info(info->wifi);
 		}

 		g_free(info->ifname);
-		g_free(info->ssid);
 		g_free(info);
 		return;
 	}
@@ -3483,14 +3481,10 @@ static void sta_remove_callback(int result,
 		info->wifi->tethering = false;
 		connman_technology_tethering_notify(info->technology, false);

-		if (info->wifi->ap_supported == WIFI_AP_SUPPORTED) {
-			g_free(info->wifi->tethering_param->ssid);
-			g_free(info->wifi->tethering_param);
-			info->wifi->tethering_param = NULL;
-		}
+		if (info->wifi->ap_supported == WIFI_AP_SUPPORTED)
+			wifi_data_free_tethering_info(info->wifi);

 		g_free(info->ifname);
-		g_free(info->ssid);
 		g_free(info);
 		return;
 	}
@@ -3563,9 +3557,6 @@ static int enable_wifi_tethering(struct connman_technology *technology,
 		info->ifname = g_strdup(ifname);

 		wifi->tethering_param->technology = technology;
-		wifi->tethering_param->ssid = ssid_ap_init(technology);
-		if (!wifi->tethering_param->ssid)
-			goto failed;

 		info->wifi->tethering = true;
 		info->wifi->ap_supported = WIFI_AP_SUPPORTED;
@@ -3584,10 +3575,13 @@ static int enable_wifi_tethering(struct connman_technology *technology,

 	failed:
 		g_free(info->ifname);
-		g_free(info->ssid);
+		if (info->ssid) {
+			g_free((char *)info->ssid->ssid);
+			g_free((char *)info->ssid->passphrase);
+			g_free(info->ssid);
+		}
 		g_free(info);
-		g_free(wifi->tethering_param);
-		wifi->tethering_param = NULL;
+		wifi_data_free_tethering_info(wifi);

 		/*
 		 * Remove bridge if it was correctly created but remove
--
2.34.1