From 72433a10f4e3e66d39ef9480957a7528d8228e5c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 29 Jul 2021 14:31:38 +0200 Subject: [PATCH] cli: fix leak of text for libreadline Coverity warns about this: Error: RESOURCE_LEAK (CWE-772): NetworkManager-1.32.4/src/nmcli/agent.c:87: alloc_fn: Storage is returned from allocation function "g_strdup". NetworkManager-1.32.4/src/nmcli/agent.c:87: var_assign: Assigning: "pre_input_deftext" = storage returned from "g_strdup(secret->value)". NetworkManager-1.32.4/src/nmcli/agent.c:87: overwrite_var: Overwriting "pre_input_deftext" in "pre_input_deftext = g_strdup(secret->value)" leaks the storage that "pre_input_deftext" points to. # 85| /* Prefill the password if we have it. */ # 86| rl_startup_hook = set_deftext; # 87|-> pre_input_deftext = g_strdup(secret->value); # 88| } # 89| if (secret->no_prompt_entry_id) Error: RESOURCE_LEAK (CWE-772): NetworkManager-1.32.4/src/nmcli/common.c:712: alloc_fn: Storage is returned from allocation function "g_strdup". NetworkManager-1.32.4/src/nmcli/common.c:712: var_assign: Assigning: "nmc_rl_pre_input_deftext" = storage returned from "g_strdup(secret->value)". NetworkManager-1.32.4/src/nmcli/common.c:712: overwrite_var: Overwriting "nmc_rl_pre_input_deftext" in "nmc_rl_pre_input_deftext = g_strdup(secret->value)" leaks the storage that "nmc_rl_pre_input_deftext" points to. # 710| /* Prefill the password if we have it. */ # 711| rl_startup_hook = nmc_rl_set_deftext; # 712|-> nmc_rl_pre_input_deftext = g_strdup(secret->value); # 713| } # 714| } --- src/nmcli/agent.c | 13 ++++++------- src/nmcli/common.c | 10 ++++------ src/nmcli/connections.c | 5 +++-- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/nmcli/agent.c b/src/nmcli/agent.c index 842843995..4801c6dd2 100644 --- a/src/nmcli/agent.c +++ b/src/nmcli/agent.c @@ -54,15 +54,14 @@ usage_agent_all(void) "Runs nmcli as both NetworkManager secret and a polkit agent.\n\n")); } -/* for pre-filling a string to readline prompt */ static char *pre_input_deftext; -static int set_deftext(_NMC_RL_STARTUPHOOK_ARGS) + +static int set_deftext(_NMC_RL_STARTUPHOOK_ARGS) { if (pre_input_deftext && rl_startup_hook) { rl_insert_text(pre_input_deftext); - g_free(pre_input_deftext); - pre_input_deftext = NULL; - rl_startup_hook = NULL; + nm_clear_g_free(&pre_input_deftext); + rl_startup_hook = NULL; } return 0; } @@ -85,8 +84,8 @@ get_secrets_from_user(const NmcConfig *nmc_config, g_print("%s\n", msg); if (secret->value) { /* Prefill the password if we have it. */ - rl_startup_hook = set_deftext; - pre_input_deftext = g_strdup(secret->value); + rl_startup_hook = set_deftext; + nm_utils_strdup_reset(&pre_input_deftext, secret->value); } if (secret->no_prompt_entry_id) pwd = nmc_readline(nmc_config, "%s: ", secret->pretty_name); diff --git a/src/nmcli/common.c b/src/nmcli/common.c index 1f9707045..a6f5cf877 100644 --- a/src/nmcli/common.c +++ b/src/nmcli/common.c @@ -711,8 +711,8 @@ get_secrets_from_user(const NmcConfig *nmc_config, continue; } else { /* Prefill the password if we have it. */ - rl_startup_hook = nmc_rl_set_deftext; - nmc_rl_pre_input_deftext = g_strdup(secret->value); + rl_startup_hook = nmc_rl_set_deftext; + nm_utils_strdup_reset(&nmc_rl_pre_input_deftext, secret->value); } } if (msg) @@ -1150,16 +1150,14 @@ nmc_rl_gen_func_ifnames(const char *text, int state) return ret; } -/* for pre-filling a string to readline prompt */ char *nmc_rl_pre_input_deftext; int nmc_rl_set_deftext(_NMC_RL_STARTUPHOOK_ARGS) { if (nmc_rl_pre_input_deftext && rl_startup_hook) { rl_insert_text(nmc_rl_pre_input_deftext); - g_free(nmc_rl_pre_input_deftext); - nmc_rl_pre_input_deftext = NULL; - rl_startup_hook = NULL; + nm_clear_g_free(&nmc_rl_pre_input_deftext); + rl_startup_hook = NULL; } return 0; } diff --git a/src/nmcli/connections.c b/src/nmcli/connections.c index e12d3e057..98f576789 100644 --- a/src/nmcli/connections.c +++ b/src/nmcli/connections.c @@ -7377,8 +7377,9 @@ property_edit_submenu(NmCli * nmc, case NMC_EDITOR_SUB_CMD_CHANGE: rl_startup_hook = nmc_rl_set_deftext; - nmc_rl_pre_input_deftext = - nmc_setting_get_property_parsable(curr_setting, prop_name, NULL); + nm_utils_strdup_reset_take( + &nmc_rl_pre_input_deftext, + nmc_setting_get_property_parsable(curr_setting, prop_name, NULL)); prop_val_user = nmc_readline(&nmc->nmc_config, _("Edit '%s' value: "), prop_name); if (!nmc_setting_set_property(nmc->client,