strfuncs: Don't assume that strtoll-like functions preserve errno

The general pattern is that a C function that participates in `errno`
leaves `errno` unspecified on success, or sets it to a defined value
on failure. Unfortunately, it is not always clear what it means for
a `strtoll()`-like function to have failed.

If an extreme value (maximum positive or minimum negative) is returned,
then we can be confident that checking for `errno == ERANGE` is the
only way to distinguish between "the true value is the max/min value"
(`errno == 0`) and "the true value would be out of range and cannot
be returned" (`errno == ERANGE`).

Otherwise, ignore `errno`, and instead rely on checking `endptr`.

This matters because `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>
This commit is contained in:
Simon McVittie 2024-07-26 13:32:24 +01:00
parent 3aee3c4e46
commit b685b1f0c4
4 changed files with 18 additions and 8 deletions

View File

@ -1039,7 +1039,7 @@ parse_int64 (const gchar *arg_name,
arg, arg_name);
return FALSE;
}
if (errno == ERANGE)
if ((tmp == G_MININT64 || tmp == G_MAXINT64) && errno == ERANGE)
{
g_set_error (error,
G_OPTION_ERROR, G_OPTION_ERROR_BAD_VALUE,

View File

@ -3388,7 +3388,6 @@ g_ascii_string_to_signed (const gchar *str,
*/
(base == 16 &&
(str_has_sign (str) ? str_has_hex_prefix (str + 1) : str_has_hex_prefix (str))) ||
(saved_errno != 0 && saved_errno != ERANGE) ||
end_ptr == NULL ||
*end_ptr != '\0')
{
@ -3397,7 +3396,9 @@ g_ascii_string_to_signed (const gchar *str,
_("“%s” is not a signed number"), str);
return FALSE;
}
if (saved_errno == ERANGE || number < min || number > max)
if (number < min ||
number > max ||
((number == G_MININT64 || number == G_MAXINT64) && saved_errno == ERANGE))
{
gchar *min_str = g_strdup_printf ("%" G_GINT64_FORMAT, min);
gchar *max_str = g_strdup_printf ("%" G_GINT64_FORMAT, max);
@ -3492,7 +3493,6 @@ g_ascii_string_to_unsigned (const gchar *str,
* 0X.
*/
(base == 16 && str_has_hex_prefix (str)) ||
(saved_errno != 0 && saved_errno != ERANGE) ||
end_ptr == NULL ||
*end_ptr != '\0')
{
@ -3501,7 +3501,9 @@ g_ascii_string_to_unsigned (const gchar *str,
_("“%s” is not an unsigned number"), str);
return FALSE;
}
if (saved_errno == ERANGE || number < min || number > max)
if (number < min ||
number > max ||
(number == G_MAXUINT64 && saved_errno == ERANGE))
{
gchar *min_str = g_strdup_printf ("%" G_GUINT64_FORMAT, min);
gchar *max_str = g_strdup_printf ("%" G_GUINT64_FORMAT, max);

View File

@ -21,6 +21,7 @@
#include "config.h"
#include <math.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
@ -1947,7 +1948,7 @@ number_get_value (AST *ast,
errno = 0;
dbl_val = g_ascii_strtod (token, &end);
if (dbl_val != 0.0 && errno == ERANGE)
if ((dbl_val >= HUGE_VAL || dbl_val <= -HUGE_VAL) && errno == ERANGE)
{
ast_set_error (ast, error, NULL,
G_VARIANT_PARSE_ERROR_NUMBER_TOO_BIG,

View File

@ -1761,7 +1761,11 @@ check_uint64 (const gchar *str,
g_assert_true (actual == result);
g_assert_cmpstr (end, ==, endptr);
g_assert_true (err == error);
if (actual == G_MAXUINT64 || *endptr != '\0')
g_assert_true (err == error);
/* ... else strtoull() succeeded, therefore errno is unspecified and may
* have been clobbered */
}
static void
@ -1781,7 +1785,10 @@ check_int64 (const gchar *str,
g_assert_true (actual == result);
g_assert_cmpstr (end, ==, endptr);
g_assert_true (err == error);
if (actual == G_MININT64 || actual == G_MAXINT64 || *endptr != '\0')
g_assert_true (err == error);
/* ... else strtoll() succeeded, therefore errno is unspecified and may
* have been clobbered */
}
static void