From e807966b84caa809ce83c47769861c929aa84a1e Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@collabora.com>
Date: Fri, 26 Jul 2024 12:42:22 +0100
Subject: [PATCH] strfuncs: Don't let get_C_locale() clobber errno

Some callers of `g_ascii_strtoull()` and similar functions assume that
they can use this pattern, similar to what they might do for
Standard C `strtoull()`:

    errno = 0;
    result = g_ascii_strtoull (nptr, endptr, base);
    saved_errno = errno;

    if (saved_errno != 0)
      g_printerr ("error parsing %s\n", nptr);

This is based on the fact that it is non-trivial to tell whether
`strtoull()` and related functions succeeded (in which case the value
of `errno` is unspecified) or failed (in which case `errno` is valid).
For example, POSIX `strtoul(3)` suggests this pattern:

> Since 0, `ULONG_MAX`, and `ULLONG_MAX` are returned on error and are
> also valid returns on success, an application wishing to check for
> error situations should set `errno` to 0, then call `strtoul()` or
> `strtoull()`, then check `errno`.

However, `g_ascii_strtoull()` does not *only* call a function resembling
`strtoull()` (`strtoull_l()` or its reimplementation
`g_parse_long_long()`): it also calls `get_C_locale()`, which wraps
`newlocale()`. Even if `newlocale()` succeeds (which in practice we
expect and assume that it will), it is valid for it to clobber `errno`.
For example, it might attempt to open a file that only conditionally
exists, which would leave `errno` set to `ENOENT`.

This is difficult to reproduce in practice: I encountered what I
believe to be this bug when compiling GLib-based software for i386 in a
Debian 12 derivative via an Open Build Service instance, but I could
not reproduce the bug in a similar chroot environment locally, and I
also could not reproduce the bug when compiling for x86_64 or for a
Debian 10, 11 or 13 derivative on the same Open Build Service instance.
It also cannot be reproduced via the GTest framework, because
`g_test_init()` indirectly calls `g_ascii_strtoull()`, resulting in
the call to `newlocale()` already having happened by the time we enter
test code.

Resolves: https://gitlab.gnome.org/GNOME/glib/-/issues/3418
Signed-off-by: Simon McVittie <smcv@collabora.com>
---
 glib/gstrfuncs.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/glib/gstrfuncs.c b/glib/gstrfuncs.c
index 122de736c..33faa80c2 100644
--- a/glib/gstrfuncs.c
+++ b/glib/gstrfuncs.c
@@ -689,12 +689,14 @@ g_ascii_strtod (const gchar *nptr,
                 gchar      **endptr)
 {
 #if defined(USE_XLOCALE) && defined(HAVE_STRTOD_L)
+  locale_t c_locale;
 
   g_return_val_if_fail (nptr != NULL, 0);
 
+  c_locale = get_C_locale ();
   errno = 0;
 
-  return strtod_l (nptr, endptr, get_C_locale ());
+  return strtod_l (nptr, endptr, c_locale);
 
 #else
 
@@ -1176,7 +1178,10 @@ g_ascii_strtoull (const gchar *nptr,
                   guint        base)
 {
 #if defined(USE_XLOCALE) && defined(HAVE_STRTOULL_L)
-  return strtoull_l (nptr, endptr, base, get_C_locale ());
+  locale_t c_locale = get_C_locale ();
+
+  errno = 0;
+  return strtoull_l (nptr, endptr, base, c_locale);
 #else
   gboolean negative;
   guint64 result;
@@ -1224,7 +1229,10 @@ g_ascii_strtoll (const gchar *nptr,
                  guint        base)
 {
 #if defined(USE_XLOCALE) && defined(HAVE_STRTOLL_L)
-  return strtoll_l (nptr, endptr, base, get_C_locale ());
+  locale_t c_locale = get_C_locale ();
+
+  errno = 0;
+  return strtoll_l (nptr, endptr, base, c_locale);
 #else
   gboolean negative;
   guint64 result;