Hi Paul, On 3/11/23 20:29, Paul Eggert wrote: > From 70985857d6d24262fc57a10bd62e6dbc642dda70 Mon Sep 17 00:00:00 2001 > From: Paul Eggert <[email protected]> > Date: Sat, 11 Mar 2023 10:07:32 -0800 > Subject: [PATCH 5/6] Fix is_my_tty overruns and truncations > > * libmisc/utmp.c: Include mempcpy.h. > (is_my_tty): Declare the parameter as a char array not char *, > as it is not necessarily null-terminated. Avoid a read overrun > when reading ut_utname. Do not silently truncate the string > returned by ttyname; instead, do not cache an overlong ttyname, > as the behavior is correct in this case (albeit slower). > Use snprintf instead of strlcpy as the latter doesn't buy much here > and this avoids depending on strlcpy. > > Signed-off-by: Paul Eggert <[email protected]> > --- > libmisc/utmp.c | 50 ++++++++++++++++++++++++++++---------------------- > 1 file changed, 28 insertions(+), 22 deletions(-) > > diff --git a/libmisc/utmp.c b/libmisc/utmp.c > index ff6acee0..9d40470e 100644 > --- a/libmisc/utmp.c > +++ b/libmisc/utmp.c > @@ -21,39 +21,45 @@ > #include <stdio.h> > > #include "alloc.h" > +#include "mempcpy.h" > > #ident "$Id$" > > +enum { UT_LINE_LEN = sizeof (getutent ()->ut_line) }; > > /* > * is_my_tty -- determine if "tty" is the same TTY stdin is using > */ > -static bool is_my_tty (const char *tty) > +static bool is_my_tty (const char tty[UT_LINE_LEN]) > { > - /* full_tty shall be at least sizeof utmp.ut_line + 5 */ > - char full_tty[200]; > - /* tmptty shall be bigger than full_tty */ > - static char tmptty[sizeof (full_tty)+1]; > - > - if ('/' != *tty) { > - (void) snprintf (full_tty, sizeof full_tty, "/dev/%s", tty); > - tty = &full_tty[0]; > - } > - > - if ('\0' == tmptty[0]) { > - const char *tname = ttyname (STDIN_FILENO); > - if (NULL != tname) > - (void) strlcpy (tmptty, tname, sizeof tmptty); > - } > - > + /* A null-terminated copy of tty, prefixed with "/dev/" if tty > + is not absolute. There is no need for snprintf, as sprintf > + cannot overrun. */ > + char full_tty[sizeof "/dev/" + UT_LINE_LEN]; > + (void) sprintf (('/' == *tty > + ? full_tty
I think it might be easier to read if we conditionally call stpcpy(3),
and then a simple sprintf(3) catenated to it.
> + : mempcpy (full_tty, "/dev/", sizeof "/dev/" - 1)),
This is a great use case for stpcpy(3). It's in POSIX.1-2008, which is
a base requirement for shadow since recently, so we can use it.
Cheers,
Alex
> + "%.*s", UT_LINE_LEN, tty);
> +
> + /* Cache of ttyname, valid if tmptty[0]. */
> + static char tmptty[UT_LINE_LEN + 1];
> +
> + const char *tname;
> if ('\0' == tmptty[0]) {
> - (void) puts (_("Unable to determine your tty name."));
> - exit (EXIT_FAILURE);
> - } else if (strncmp (tty, tmptty, sizeof (tmptty)) != 0) {
> - return false;
> + tname = ttyname (STDIN_FILENO);
> + if (! tname) {
> + (void) puts (_("Unable to determine your tty name."));
> + exit (EXIT_FAILURE);
> + }
> + int tnamelen = snprintf (tmptty, sizeof tmptty, "%s", tname);
> + if (! (0 <= tnamelen && tnamelen < sizeof tmptty)) {
> + tmptty[0] = '\0';
> + }
> } else {
> - return true;
> + tname = tmptty;
> }
> +
> + return strcmp (full_tty, tname) == 0;
> }
>
> /*
> --
> 2.37.2
>
--
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5
OpenPGP_signature
Description: OpenPGP digital signature
_______________________________________________ Pkg-shadow-devel mailing list [email protected] https://alioth-lists.debian.net/cgi-bin/mailman/listinfo/pkg-shadow-devel
