[patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)

[patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)

[Marcin Stolarek]

[-- Attachment #1: Type: text/plain, Size: 1003 bytes --]

The change in glibc commit d58ab810a6e325cc351684d174c48cabce01bcc1
(author in CC):

From commit description:"[...] Also avoid an unnecessary call to
strcspn after the last token by adding an early exit for an empty
string.[...]"

Important code change:
/* Parse S into tokens separated by characters in DELIM.
@@ -45,11 +41,17 @@
 char *
 __strtok_r (char *s, const char *delim, char **save_ptr)
 {
-  char *token;
+  char *end;

   if (s == NULL)
     s = *save_ptr;

+  if (*s == '\0')
+    {
+      *save_ptr = s;
+      return NULL;
+    }
+

may result in the mentioned segmentation fault if the char *str passed
to strtok_r is a NULL (for 1st call).

Checked glibc versions:
~/git-repos/glibc:release/2.25/master$ git tag
--contain=d58ab810a6e325cc351684d174c48cabce01bcc1
changelog-ends-here
glibc-2.25
glibc-2.25.90
glibc-2.26
glibc-2.26.9000
glibc-2.27
glibc-2.27.9000
glibc-2.28
glibc-2.28.9000
glibc-2.29
glibc-2.29.9000
glibc-2.30
glibc-2.30.9000
glibc-2.31
glibc-2.31.9000

cheers,
Marcin

[-- Attachment #2: 0001-Add-note-about-strtok_r-change-in-glibc-2.25.patch --]
[-- Type: text/x-patch, Size: 936 bytes --]

From cdbe9daffdb36400c23a1cb47acd7252d2ad434a Mon Sep 17 00:00:00 2001
From: Marcin Stolarek <stolarek.marcin@gmail.com>
Date: Sat, 25 Apr 2020 14:41:24 +0200
Subject: [PATCH 1/1] Add note about strtok_r() change in glibc-2.25

Calling strtok(NULL,',',NULL) after d58ab810a6e325cc351684d174c48cabce01bcc1
will result in NULL pointer dereference.
---
 man3/strtok.3 | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/man3/strtok.3 b/man3/strtok.3
index 933a7b96c..8754a0216 100644
--- a/man3/strtok.3
+++ b/man3/strtok.3
@@ -197,6 +197,11 @@ is required to be NULL on the first call to
 .BR strtok_r ()
 that is being used to parse
 .IR str .
+.\" glibc-2.25 d58ab810a6e325cc351684d174c48cabce01bcc1
+Since glibc-2.25 using
+.BR strtok_r ()
+with str set to NULL (for the 1st call) is not allowed and may result in
+segmentation fault.
 .SH BUGS
 Be cautious when using these functions.
 If you do use them, note that:
-- 
2.17.1

Re: [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)

[Michael Kerrisk (man-pages)]

On Sat, 25 Apr 2020 at 14:52, Marcin Stolarek <stolarek.marcin@gmail.com> wrote:
>
> The change in glibc commit d58ab810a6e325cc351684d174c48cabce01bcc1
> (author in CC):
>
> From commit description:"[...] Also avoid an unnecessary call to
> strcspn after the last token by adding an early exit for an empty
> string.[...]"
>
> Important code change:
> /* Parse S into tokens separated by characters in DELIM.
> @@ -45,11 +41,17 @@
>  char *
>  __strtok_r (char *s, const char *delim, char **save_ptr)
>  {
> -  char *token;
> +  char *end;
>
>    if (s == NULL)
>      s = *save_ptr;
>
> +  if (*s == '\0')
> +    {
> +      *save_ptr = s;
> +      return NULL;
> +    }
> +
>
> may result in the mentioned segmentation fault if the char *str passed
> to strtok_r is a NULL (for 1st call).

I'm a little puzzled here. Getting a segfault if the first argument to
strtok_r() is NULL on the first call seems not so surprising to me.
Why would you expect otherwise? What am I missing?

Thanks,

Michael

Re: [patch] strtok.3 add note about glibc-2.25+ segfault for call like strtok_r(NULL,"|",NULL)

[Wilco Dijkstra]

Hi,

> It worked fine prior to 2.25, since it's a change I thoight it may be nice to document that this set
> of arguments will segfault and has to be avoided by calling app itself.

Did it ever? Even much older versions have always accessed *save_ptr with a NULL input, so a crash
is guaranteed. And *s is also always accessed, so it had better never be NULL. Eg. this is what it did
back in 2007 ([1]):

  if (s == NULL)
    s = *save_ptr;  // crash here

  /* Scan leading delimiters.  */
  s += strspn (s, delim);
  if (*s == '\0')
    {
      *save_ptr = s;
      return NULL;
    }

Cheers,
Wilco

[1] https://github.com/bminor/glibc/blob/0ecb606cb6cf65de1d9fc8a919bceb4be476c602/string/strtok_r.c

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.