public inbox for newlib@sourceware.org
 help / color / mirror / Atom feed
From: Torbjorn SVENSSON <torbjorn.svensson@foss.st.com>
To: <newlib@sourceware.org>
Subject: Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty
Date: Wed, 12 Jul 2023 20:50:25 +0200	[thread overview]
Message-ID: <74633613-8420-5ad6-2882-6ad14066f781@foss.st.com> (raw)
In-Reply-To: <1de3b3ee-7dd8-db16-6e17-365dbd9fde84@fibranet.cat>



On 2023-07-12 19:33, Jordi Sanfeliu via Newlib wrote:
> Hello,
> 
> In my hobby OS [1] which uses Newlib C as its libc, I noticed that the 
> GNU command 'logname' does output nothing when it is redirected or pipe'd.
> 
> The current getlogin() implementation [2] forces the three primary file 
> descriptors (stdin, stdout and stderr) to be a valid tty before checking 
> the utmp file, otherwise it returns NULL. This makes impossible to 
> redirect (to a file or to a pipe), the output of a program that is using 
> this function because one of its file descriptors won't be a tty.

I think your analysis is wrong. See below for reasons why.

> diff --git a/newlib/libc/unix/getlogin.c b/newlib/libc/unix/getlogin.c
> index da4f47a95..e646bcb08 100644
> --- a/newlib/libc/unix/getlogin.c
> +++ b/newlib/libc/unix/getlogin.c
> @@ -16,9 +16,7 @@ getlogin ()
>     extern char *ttyname ();
>     char *tty;
> 
> -  if (((tty = ttyname (0)) == 0)
> -      || ((tty = ttyname (1)) == 0)
> -      || ((tty = ttyname (2)) == 0))

These 3 lines of code checks if one of stdin, stdout or stderr is 
connected to a terminal device. If the return value of ttyname is 0, it 
means that there is no terminal device connected to that fd.
As I read the code, it first tries with stdin. If stdin is closed or 
redirected, it tries with stdout instead and then lastly, falls back to 
trying with stderr. If none of the 3 fd's provides a terminal device, 
then the getlogin will return 0.

As a result, doing your change would force the process to have a 
connected stdin or the getlogin call would return 0.

> +  if ((tty = ttyname (0)) == 0)
>       return 0;
> 
>     if ((utmp_fd = open (UTMP_FILE, O_RDONLY)) == -1)
> 
> 
> 1. https://www.fiwix.org
> 2. 
> https://sourceware.org/git/?p=newlib-cygwin.git;a=blob_plain;f=newlib/libc/unix/getlogin.c;hb=HEAD
> 


While looking at the code pointed to in link 2, I'm a bit puzzled about 
the following lines:

static char buf[10];
...
strncpy (buf, utmp_buf.ut_user, sizeof (utmp_buf.ut_user));

As buf is 10 bytes, I suppose sizeof(utmp_buf.ut_user) should never be 
allowed to exceed 10 bytes, but in the newlib source tree, I find these 
files that define a size for utmp_buf.ut_user:

./newlib/libc/sys/sysvi386/sys/utmp.h: 8
./winsup/cygwin/include/cygwin/utmp.h: 16

I'm not sure if this getlogin.c file is included in a cygwin build, but 
if it is, I think there is a buffer overflow here.


Kind regards,
Torbjörn

  reply	other threads:[~2023-07-12 18:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-12 17:33 Jordi Sanfeliu
2023-07-12 18:50 ` Torbjorn SVENSSON [this message]
2023-07-12 20:06   ` Brian Inglis
2023-07-13 16:26     ` Torbjorn SVENSSON
2023-07-13  8:06   ` Jordi Sanfeliu
2023-07-13 16:25     ` Torbjorn SVENSSON
2023-07-13 16:57       ` Jordi Sanfeliu
2023-07-13 19:01         ` Jeff Johnston
2023-07-17  7:54           ` Corinna Vinschen
2023-07-13 21:12         ` Stefan Tauner
2023-07-17 19:06 ` Jeff Johnston
2023-07-18  5:57   ` Jordi Sanfeliu
2023-07-18  7:08   ` Jordi Sanfeliu
2023-07-18 17:44     ` Jeff Johnston
2023-07-18 18:59       ` Jordi Sanfeliu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=74633613-8420-5ad6-2882-6ad14066f781@foss.st.com \
    --to=torbjorn.svensson@foss.st.com \
    --cc=newlib@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).