Hi Paul,
On 3/11/23 20:29, Paul Eggert wrote:
> From 70985857d6d24262fc57a10bd62e6dbc642dda70 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <egg...@cs.ucla.edu>
> 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 <egg...@cs.ucla.edu>
> ---
> 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
