From 62c1944e7dcc22adbeb5b25bb9bbd268a1cccf16 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 25 May 2021 14:14:26 +0200
Subject: [PATCH] libnm: fix logic and double free in
 nm_vpn_service_plugin_read_vpn_details()

"val" and "key" are now marked as nm_auto. They are freed at the end,
and we should not free them before breaking the loop (at least not,
without also clearing the variables).

Fixes: 02dbba49d6d3 ('libnm: fix leak in nm_vpn_service_plugin_read_vpn_details()')
---
 src/libnm-client-impl/nm-vpn-service-plugin.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/libnm-client-impl/nm-vpn-service-plugin.c b/src/libnm-client-impl/nm-vpn-service-plugin.c
index 016c761c40..1188924afc 100644
--- a/src/libnm-client-impl/nm-vpn-service-plugin.c
+++ b/src/libnm-client-impl/nm-vpn-service-plugin.c
@@ -759,8 +759,7 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
     nm_auto_free_gstring GString *val      = NULL;
     nm_auto_free_gstring GString *line     = NULL;
     char                          c;
-
-    GString *str = NULL;
+    GString *                     str = NULL;
 
     if (out_data)
         g_return_val_if_fail(*out_data == NULL, FALSE);
@@ -808,16 +807,12 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
             /* finish marker */
             break;
         } else if (strncmp(line->str, DATA_KEY_TAG, strlen(DATA_KEY_TAG)) == 0) {
-            if (key != NULL) {
+            if (nm_clear_g_string(&key))
                 g_warning("a value expected");
-                g_string_free(key, TRUE);
-            }
             key  = g_string_new(line->str + strlen(DATA_KEY_TAG));
             str  = key;
             hash = data;
         } else if (strncmp(line->str, DATA_VAL_TAG, strlen(DATA_VAL_TAG)) == 0) {
-            if (val != NULL)
-                g_string_free(val, TRUE);
             if (val || !key || hash != data) {
                 g_warning("%s not preceded by %s", DATA_VAL_TAG, DATA_KEY_TAG);
                 break;
@@ -825,16 +820,12 @@ nm_vpn_service_plugin_read_vpn_details(int fd, GHashTable **out_data, GHashTable
             val = g_string_new(line->str + strlen(DATA_VAL_TAG));
             str = val;
         } else if (strncmp(line->str, SECRET_KEY_TAG, strlen(SECRET_KEY_TAG)) == 0) {
-            if (key != NULL) {
+            if (nm_clear_g_string(&key))
                 g_warning("a value expected");
-                g_string_free(key, TRUE);
-            }
             key  = g_string_new(line->str + strlen(SECRET_KEY_TAG));
             str  = key;
             hash = secrets;
         } else if (strncmp(line->str, SECRET_VAL_TAG, strlen(SECRET_VAL_TAG)) == 0) {
-            if (val != NULL)
-                g_string_free(val, TRUE);
             if (val || !key || hash != secrets) {
                 g_warning("%s not preceded by %s", SECRET_VAL_TAG, SECRET_KEY_TAG);
                 break;