Merge branch '3187-vasprintf-checks' into 'main'

gprintf: Fix invalid size allocation in `g_vasprintf()`

Closes #3187

See merge request GNOME/glib!3729

glib/gmessages.c

@@ -3391,12 +3391,25 @@ g_printerr (const gchar *format,
  * Calculates the maximum space needed to store the output
  * of the sprintf() function.
  *
- * Returns: the maximum space needed to store the formatted string
+ * If @format or @args are invalid, `0` is returned. This could happen if, for
+ * example, @format contains an `%lc` or `%ls` placeholder and @args contains a
+ * wide character which cannot be represented in multibyte encoding. `0`
+ * can also be returned legitimately if, for example, @format is `%s` and @args
+ * is an empty string. The caller is responsible for differentiating these two
+ * return cases if necessary. It is recommended to not use `%lc` or `%ls`
+ * placeholders in any case, as their behaviour is locale-dependent.
+ *
+ * Returns: the maximum space needed to store the formatted string, or `0` on error
  */
 gsize
 g_printf_string_upper_bound (const gchar *format,
                              va_list      args)
 {
   gchar c;
-  return _g_vsnprintf (&c, 1, format, args) + 1;
+  int count = _g_vsnprintf (&c, 1, format, args);
+
+  if (count < 0)
+    return 0;
+
+  return count + 1;
 }

glib/gprintf.c

@@ -359,19 +359,29 @@ g_vasprintf (gchar      **string,
 
   {
     va_list args2;
+    char c;
+    int max_len;
 
     va_copy (args2, args);
 
-    *string = g_new (gchar, g_printf_string_upper_bound (format, args));
+    max_len = _g_vsnprintf (&c, 1, format, args);
+    if (max_len < 0)
+      {
+        /* This can happen if @format contains `%ls` or `%lc` and @args contains
+         * something not representable in the current locale’s encoding (which
+         * should be UTF-8, but ymmv). Basically: don’t use `%ls` or `%lc`. */
+        va_end (args2);
+        *string = NULL;
+        return -1;
+      }
+
+    *string = g_new (gchar, (size_t) max_len + 1);
 
     len = _g_vsprintf (*string, format, args2);
     va_end (args2);
 
-    if (len < 0)
+    /* _g_vsprintf() should have exactly the same failure modes as _g_vsnprintf() */
-      {
+    g_assert (len >= 0);
-        g_free (*string);
-        *string = NULL;
-      }
   }
 #endif
 

glib/gstrfuncs.c

@@ -542,7 +542,7 @@ g_stpcpy (gchar       *dest,
  * See also g_vasprintf(), which offers the same functionality, but
  * additionally returns the length of the allocated string.
  *
- * Returns: a newly-allocated string holding the result
+ * Returns: (nullable) (transfer full): a newly-allocated string holding the result
  */
 gchar*
 g_strdup_vprintf (const gchar *format,
@@ -570,7 +570,7 @@ g_strdup_vprintf (const gchar *format,
  * contains `%lc` or `%ls` conversions, which can fail if no multibyte
  * representation is available for the given character.
  *
- * Returns: a newly-allocated string holding the result
+ * Returns: (nullable) (transfer full): a newly-allocated string holding the result
  */
 gchar*
 g_strdup_printf (const gchar *format,

glib/gstring.c

@@ -1221,6 +1221,10 @@ g_string_append_vprintf (GString     *string,
       string->len += len;
       g_free (buf);
     }
+  else
+    {
+      g_critical ("Failed to append to string: invalid format/args passed to g_vasprintf()");
+    }
 }
 
 /**

glib/tests/string.c

@@ -319,11 +319,17 @@ test_string_append_vprintf (void)
 
   string_append_vprintf_va (string, "some %s placeholders", "format");
 
+  if (g_test_undefined ())
+    {
+      g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_CRITICAL,
+                             "Failed to append to string*");
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wformat"
 #pragma GCC diagnostic ignored "-Wformat-extra-args"
   string_append_vprintf_va (string, "%l", "invalid");
 #pragma GCC diagnostic pop
+      g_test_assert_expected_messages ();
+    }
 
   g_assert_cmpstr (string->str, ==, "firsthalfsome format placeholders");
 

glib/tests/test-printf.c

@@ -897,6 +897,9 @@ test_upper_bound (void)
 
   res = upper_bound ("bla %s %d: %g\n", "bla", 123, 0.123);
   g_assert_cmpint (res, ==, 20);
+
+  res = upper_bound ("Invalid case: %ls", L"\xD800" /* incomplete surrogate pair */);
+  g_assert_cmpint (res, ==, 0);
 }
 
 #if !defined(__APPLE__) && !defined(__FreeBSD__)
@@ -945,6 +948,30 @@ test_vasprintf_invalid_format_placeholder (void)
 #endif
 }
 
+static void
+test_vasprintf_invalid_wide_string (void)
+{
+#if !defined(__APPLE__) && !defined(__FreeBSD__)
+  gint len = 0;
+  gchar *buf = "some non-null string";
+#endif
+
+  g_test_summary ("Test error handling for invalid wide strings in g_vasprintf()");
+
+#if !defined(__APPLE__) && !defined(__FreeBSD__)
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wformat"
+#pragma GCC diagnostic ignored "-Wformat-extra-args"
+  len = test_vasprintf_va (&buf, "%ls", L"\xD800"  /* incomplete surrogate pair */);
+#pragma GCC diagnostic pop
+
+  g_assert_cmpint (len, ==, -1);
+  g_assert_null (buf);
+#else
+  g_test_skip ("vasprintf() placeholder checks on BSDs are less strict");
+#endif
+}
+
 int
 main (int   argc,
       char *argv[])
@@ -986,6 +1013,7 @@ main (int   argc,
   g_test_add_func ("/sprintf/upper-bound", test_upper_bound);
 
   g_test_add_func ("/vasprintf/invalid-format-placeholder", test_vasprintf_invalid_format_placeholder);
+  g_test_add_func ("/vasprintf/invalid-wide-string", test_vasprintf_invalid_wide_string);
 
   return g_test_run();
 }