From 725cc687106d6450bf6d6878e7d2936ddc56bca9 Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Wed, 18 Dec 2019 13:42:06 +0100 Subject: [PATCH 1/2] common: readline: fix memory leak of plain text secret After a user entered a secret it would get stored in the readline history data structure (in plain text) and eventually get leaked. This commit instructs readline to not store any secret in its history and fixes a non-related memory leak. --- clients/cli/common.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/clients/cli/common.c b/clients/cli/common.c index b5e684cecb..58879f6a51 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -1005,7 +1005,7 @@ nmc_readline_echo (const NmcConfig *nmc_config, va_list args; gs_free char *prompt = NULL; char *str; - HISTORY_STATE *saved_history; + nm_auto_free HISTORY_STATE *saved_history = NULL; HISTORY_STATE passwd_history = { 0, }; va_start (args, prompt_fmt); @@ -1018,6 +1018,10 @@ nmc_readline_echo (const NmcConfig *nmc_config, if (!echo_on) { saved_history = history_get_history_state (); history_set_history_state (&passwd_history); + /* stifling history is important as it tells readline to + * not store anything, otherwise sensitive data could be + * leaked */ + stifle_history (0); rl_redisplay_function = nmc_secret_redisplay; } From 730adf2afd8be6c3c44cc5a9b7ce64245bf8f1fc Mon Sep 17 00:00:00 2001 From: Antonio Cardace Date: Wed, 18 Dec 2019 14:46:22 +0100 Subject: [PATCH 2/2] clients,libnm-core: zero-out memory used to store plain-text secrets --- clients/cli/common.c | 3 ++- clients/cli/devices.c | 3 ++- libnm-core/nm-setting-wireless-security.c | 25 ++++++++++++----------- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/clients/cli/common.c b/clients/cli/common.c index 58879f6a51..452f04f624 100644 --- a/clients/cli/common.c +++ b/clients/cli/common.c @@ -17,6 +17,7 @@ #include "nm-vpn-helpers.h" #include "nm-client-utils.h" +#include "nm-glib-aux/nm-secret-utils.h" #include "utils.h" @@ -732,7 +733,7 @@ get_secrets_from_user (const NmcConfig *nmc_config, /* No password provided, cancel the secrets. */ if (!pwd) return FALSE; - g_free (secret->value); + nm_free_secret (secret->value); secret->value = pwd; } return TRUE; diff --git a/clients/cli/devices.c b/clients/cli/devices.c index 0a7339bc2c..a23113339c 100644 --- a/clients/cli/devices.c +++ b/clients/cli/devices.c @@ -18,6 +18,7 @@ #include "utils.h" #include "common.h" #include "connections.h" +#include "nm-glib-aux/nm-secret-utils.h" /* define some prompts */ #define PROMPT_INTERFACE _("Interface: ") @@ -3639,7 +3640,7 @@ finish: if (bssid2_arr) g_byte_array_free (bssid2_arr, TRUE); g_free (ssid_ask); - g_free (passwd_ask); + nm_free_secret (passwd_ask); return nmc->return_value; } diff --git a/libnm-core/nm-setting-wireless-security.c b/libnm-core/nm-setting-wireless-security.c index b0b96f9957..6032053bbb 100644 --- a/libnm-core/nm-setting-wireless-security.c +++ b/libnm-core/nm-setting-wireless-security.c @@ -13,6 +13,7 @@ #include "nm-utils-private.h" #include "nm-setting-private.h" #include "nm-setting-wireless.h" +#include "nm-glib-aux/nm-secret-utils.h" /** * SECTION:nm-setting-wireless-security @@ -1316,33 +1317,33 @@ set_property (GObject *object, guint prop_id, priv->leap_username = g_value_dup_string (value); break; case PROP_WEP_KEY0: - g_free (priv->wep_key0); + nm_free_secret (priv->wep_key0); priv->wep_key0 = g_value_dup_string (value); break; case PROP_WEP_KEY1: - g_free (priv->wep_key1); + nm_free_secret (priv->wep_key1); priv->wep_key1 = g_value_dup_string (value); break; case PROP_WEP_KEY2: - g_free (priv->wep_key2); + nm_free_secret (priv->wep_key2); priv->wep_key2 = g_value_dup_string (value); break; case PROP_WEP_KEY3: - g_free (priv->wep_key3); + nm_free_secret (priv->wep_key3); priv->wep_key3 = g_value_dup_string (value); break; case PROP_WEP_KEY_FLAGS: priv->wep_key_flags = g_value_get_flags (value); break; case PROP_PSK: - g_free (priv->psk); + nm_free_secret (priv->psk); priv->psk = g_value_dup_string (value); break; case PROP_PSK_FLAGS: priv->psk_flags = g_value_get_flags (value); break; case PROP_LEAP_PASSWORD: - g_free (priv->leap_password); + nm_free_secret (priv->leap_password); priv->leap_password = g_value_dup_string (value); break; case PROP_LEAP_PASSWORD_FLAGS: @@ -1392,12 +1393,12 @@ finalize (GObject *object) g_free (priv->key_mgmt); g_free (priv->auth_alg); g_free (priv->leap_username); - g_free (priv->wep_key0); - g_free (priv->wep_key1); - g_free (priv->wep_key2); - g_free (priv->wep_key3); - g_free (priv->psk); - g_free (priv->leap_password); + nm_free_secret (priv->wep_key0); + nm_free_secret (priv->wep_key1); + nm_free_secret (priv->wep_key2); + nm_free_secret (priv->wep_key3); + nm_free_secret (priv->psk); + nm_free_secret (priv->leap_password); g_slist_free_full (priv->proto, g_free); g_slist_free_full (priv->pairwise, g_free);