[Pkg-shadow-devel] Bug#1032393: Bug#1032393: [PATCH v2 2/2] debian/control: Add libbsd-dev and pkg-config

[Alejandro Colomar]

Hi Paul,

On 3/11/23 20:29, Paul Eggert wrote:
> From 70985857d6d24262fc57a10bd62e6dbc642dda70 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert at 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 <eggert at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <http://alioth-lists.debian.net/pipermail/pkg-shadow-devel/attachments/20230311/d52c26ec/attachment-0001.sig>