POSIX leaves the value of *endptr unspecified if the base is invalid;
it is not unmodified.

glibc and musl libc leave the object unmodified.  However, the BSDs and
Bionic libc modify it to be nptr.

That, combined with the fact that POSIX allows implementations of
strtol(3) to set EINVAL when no digits are found, makes it impossible to
portably check if the base was valid after the call.

This bug was introduced after I incorrectly mentioned that strtol(3)
left *endptr unmodified; that's an implementation-defined behavior of
glibc and musl.

Fixes: 64ddc975e72c (2024-07-18, "xstrtol: document and stray less from strtol")
Fixes: 16b33e664942 (2024-07-19, "xstrtol: be more robust against odd failures")
Link: <https://mail-index.netbsd.org/tech-misc/2024/07/20/msg000412.html>
Reported-by: Robert Elz <k...@munnari.oz.au>
Cc: Paul Eggert <egg...@cs.ucla.edu>
Cc: Bruno Haible <br...@clisp.org>
Signed-off-by: Alejandro Colomar <a...@kernel.org>
---

Hi Paul, Bruno,

I haven't tested the patch.  Please check thoroughly.  I'm tired of
strtol(3).  :)

This was discussed in a NetBSD report about their strtoi(3)
implementation not being portable.  I CCed Paul in a message there.

Have a lovely day!
Alex

 lib/xstrtol.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/lib/xstrtol.c b/lib/xstrtol.c
index 797d3e4dee..ccbfa82924 100644
--- a/lib/xstrtol.c
+++ b/lib/xstrtol.c
@@ -71,9 +71,14 @@ strtol_error
 __xstrtol (char const *nptr, char **endptr, int base,
            __strtol_t *val, char const *valid_suffixes)
 {
-  char *t_ptr = nullptr;
+  char *t_ptr;
   char **p = endptr ? endptr : &t_ptr;
 
+  *p = (char *) nptr;
+
+  if (base < 0 || base == 1 || 36 < base)
+    return LONGINT_INVALID;
+
   if (! TYPE_SIGNED (__strtol_t))
     {
       char const *q = nptr;
@@ -81,20 +86,23 @@ __xstrtol (char const *nptr, char **endptr, int base,
       while (isspace (ch))
         ch = *++q;
       if (ch == '-')
-        {
-          *p = (char *) nptr;
-          return LONGINT_INVALID;
-        }
+        return LONGINT_INVALID;
     }
 
   errno = 0;
-  __strtol_t tmp = __strtol (nptr, &t_ptr, base);
-  if (!t_ptr)
-    return LONGINT_INVALID;
-  *p = t_ptr;
+  __strtol_t tmp = __strtol (nptr, p, base);
+
   strtol_error err = LONGINT_OK;
 
-  if (*p == nptr && (errno == 0 || errno == EINVAL))
+  if (errno == ERANGE)
+    {
+      err = LONGINT_OVERFLOW;
+    }
+  else if (errno != 0 && errno != EINVAL)
+    {
+      return LONGINT_INVALID;
+    }
+  else if (*p == nptr)
     {
       /* If there is no number but there is a valid suffix, assume the
          number is 1.  The string is invalid otherwise.  */
@@ -102,12 +110,6 @@ __xstrtol (char const *nptr, char **endptr, int base,
         return LONGINT_INVALID;
       tmp = 1;
     }
-  else if (errno != 0)
-    {
-      if (errno != ERANGE)
-        return LONGINT_INVALID;
-      err = LONGINT_OVERFLOW;
-    }
 
   /* Let valid_suffixes == NULL mean "allow any suffix".  */
   /* FIXME: update all callers except the ones that allow suffixes
-- 
2.45.2

Attachment: signature.asc
Description: PGP signature

Reply via email to