From 3fd1b63453417617fc1709a2d12aaa6f3691b40d Mon Sep 17 00:00:00 2001
From: Jan Luebbe <jluebbe@debian.org>
Date: Tue, 29 Aug 2023 15:32:27 +0200
Subject: [PATCH] gkeyfile: Fix overwriting of GError

When parsing a line of "key3=foo\i\" in a keyfile,
g_key_file_parse_value_as_string currently first sets the error to
'contains invalid escape' and later to 'contains escape character at end
of line'.

This leaks the first GError and causes the following warning message:
  Error set over the top of a previous GError or uninitialized memory.
  This indicates a bug in someone's code. You must ensure an error is
  NULL before it's set. The overwriting error message was: Key file
  contains escape character at end of line

Fix this by returning when an error is detected. As we may have
collected data in pieces, we instead collect to a tmp_pieces GSList and
free it on error.
---
 glib/gkeyfile.c      | 42 ++++++++++++++++++++++++++----------------
 glib/tests/keyfile.c | 25 +++++++++++++++----------
 2 files changed, 41 insertions(+), 26 deletions(-)

diff --git a/glib/gkeyfile.c b/glib/gkeyfile.c
index 9a4821bc5..c000ed2fe 100644
--- a/glib/gkeyfile.c
+++ b/glib/gkeyfile.c
@@ -4287,8 +4287,11 @@ g_key_file_parse_value_as_string (GKeyFile     *key_file,
 				  GError      **error)
 {
   gchar *string_value, *q0, *q;
+  GSList *tmp_pieces = NULL;
   const gchar *p;
 
+  g_assert (pieces == NULL || *pieces == NULL);
+
   string_value = g_new (gchar, strlen (value) + 1);
 
   p = value;
@@ -4326,7 +4329,7 @@ g_key_file_parse_value_as_string (GKeyFile     *key_file,
                                    G_KEY_FILE_ERROR_INVALID_VALUE,
                                    _("Key file contains escape character "
                                      "at end of line"));
-	      break;
+              goto error;
 
             default:
 	      if (pieces && *p == key_file->list_separator)
@@ -4348,20 +4351,21 @@ g_key_file_parse_value_as_string (GKeyFile     *key_file,
 				   G_KEY_FILE_ERROR_INVALID_VALUE,
 				   _("Key file contains invalid escape "
 				     "sequence “%s”"), sequence);
-		    }
-		}
+                      goto error;
+                    }
+                }
               break;
             }
         }
       else
-	{
-	  *q = *p;
-	  if (pieces && (*p == key_file->list_separator))
-	    {
-	      *pieces = g_slist_prepend (*pieces, g_strndup (q0, q - q0));
-	      q0 = q + 1; 
-	    }
-	}
+        {
+          *q = *p;
+          if (pieces && (*p == key_file->list_separator))
+            {
+              tmp_pieces = g_slist_prepend (tmp_pieces, g_strndup (q0, q - q0));
+              q0 = q + 1;
+            }
+        }
 
       if (*p == '\0')
 	break;
@@ -4372,13 +4376,19 @@ g_key_file_parse_value_as_string (GKeyFile     *key_file,
 
   *q = '\0';
   if (pieces)
-  {
-    if (q0 < q)
-      *pieces = g_slist_prepend (*pieces, g_strndup (q0, q - q0));
-    *pieces = g_slist_reverse (*pieces);
-  }
+    {
+      if (q0 < q)
+        tmp_pieces = g_slist_prepend (tmp_pieces, g_strndup (q0, q - q0));
+      *pieces = g_slist_reverse (tmp_pieces);
+    }
 
   return string_value;
+
+error:
+  g_free (string_value);
+  g_slist_free_full (tmp_pieces, g_free);
+
+  return NULL;
 }
 
 static gchar *
diff --git a/glib/tests/keyfile.c b/glib/tests/keyfile.c
index 3d72d9670..113b4f208 100644
--- a/glib/tests/keyfile.c
+++ b/glib/tests/keyfile.c
@@ -535,16 +535,17 @@ test_string (void)
     "3",
   };
   const gchar *data =
-    "[valid]\n"
-    "key1=\\s\\n\\t\\r\\\\\n"
-    "key2=\"quoted\"\n"
-    "key3='quoted'\n"
-    "key4=\xe2\x89\xa0\xe2\x89\xa0\n"
-    "key5=  leading space\n"
-    "key6=trailing space  \n"
-    "[invalid]\n"
-    "key1=\\a\\b\\0800xff\n"
-    "key2=blabla\\\n";
+      "[valid]\n"
+      "key1=\\s\\n\\t\\r\\\\\n"
+      "key2=\"quoted\"\n"
+      "key3='quoted'\n"
+      "key4=\xe2\x89\xa0\xe2\x89\xa0\n"
+      "key5=  leading space\n"
+      "key6=trailing space  \n"
+      "[invalid]\n"
+      "key1=\\a\\b\\0800xff\n"
+      "key2=blabla\\\n"
+      "key3=foo\\i\\\n";
 
   keyfile = load_data (data, 0);
 
@@ -563,6 +564,10 @@ test_string (void)
   check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE);
   g_free (value);
 
+  value = g_key_file_get_string (keyfile, "invalid", "key3", &error);
+  check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_INVALID_VALUE);
+  g_free (value);
+
   g_key_file_set_string (keyfile, "inserted", "key1", "simple");
   g_key_file_set_string (keyfile, "inserted", "key2", " leading space");
   g_key_file_set_string (keyfile, "inserted", "key3", "\tleading tab");