Re: coreutils readutmp applies strchr to a non-string

[Charlie Gordon]

When are we going to stop using strncpy !
this C library function is a mess, it doesn't do what most C programmers
believe.
It causes bugs like this one, or blatant inefficiencies due to the inept
null padding on large buffers.
I bet there are quite a few problems around uses of this function even in
gnu software.

Other candidates are : sprintf, gets, strtok, mktemp, tmpnam,
tempnam...do/while loops...
 or any of the C library functions duly tagged in the man pages as never to
be used.

Chqrlie

PS: the non believers can pull up the man page for strncpy and read it
carefully.
It is so unlikely that the precise behaviour of that horrible thing be what
is needed in any C program...


"Paul Eggert" <address@hidden> wrote in message
news:address@hidden
> coreutils readutmp applies strchr to a buffer that is not
> null-terminated.  This has undefined behavior in C, even if the buffer
> has the char in question.  Here's a patch.
>
> 2004-06-11  Paul Eggert  <address@hidden>
>
> * readutmp.c (extract_trimmed_name): Don't apply strchr to a
> non-string; this leads to undefined behavior.
>
> Index: readutmp.c
> ===================================================================
> RCS file: /home/meyering/coreutils/cu/lib/readutmp.c,v
> retrieving revision 1.19
> diff -p -u -r1.19 readutmp.c
> --- readutmp.c 19 Apr 2004 18:59:52 -0000 1.19
> +++ readutmp.c 12 Jun 2004 01:21:31 -0000
> @@ -40,12 +40,14 @@ extract_trimmed_name (const STRUCT_UTMP
>
>    trimmed_name = xmalloc (sizeof (UT_USER (ut)) + 1);
>    strncpy (trimmed_name, UT_USER (ut), sizeof (UT_USER (ut)));
> -  /* Append a trailing space character.  Some systems pad names shorter
than
> -     the maximum with spaces, others pad with NULs.  Remove any spaces.
*/
> -  trimmed_name[sizeof (UT_USER (ut))] = ' ';
> -  p = strchr (trimmed_name, ' ');
> -  if (p != NULL)
> -    *p = '\0';
> +  /* Append a trailing NUL.  Some systems pad names shorter than the
> +     maximum with spaces, others pad with NULs.  Remove any trailing
> +     spaces.  */
> +  trimmed_name[sizeof (UT_USER (ut))] = '\0';
> +  for (p = trimmed_name + strlen (trimmed_name);
> +       trimmed_name < p && p[-1] == ' ';
> +       *--p = '\0')
> +    continue;
>    return trimmed_name;
>  }