From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx07-00178001.pphosted.com (mx08-00178001.pphosted.com [91.207.212.93]) by sourceware.org (Postfix) with ESMTPS id 1D2B03858D20 for ; Wed, 12 Jul 2023 18:50:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 1D2B03858D20 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=foss.st.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=foss.st.com Received: from pps.filterd (m0046660.ppops.net [127.0.0.1]) by mx07-00178001.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 36CF8cTS018073 for ; Wed, 12 Jul 2023 20:50:28 +0200 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=foss.st.com; h=message-id : date : mime-version : subject : to : references : from : in-reply-to : content-type : content-transfer-encoding; s=selector1; bh=71Cm9sHgMifmb3LJihp2Js9JfC8mNdX21MGe3Fit8bA=; b=Z5xjDt0i25yik/BqCOAcaWgTxKjQ5OsUwOPTtdhLVHHemYhAR3lMGgfNKQKpWHIuZgFV UNh1E6sZAlh0s4tEnmdHbLCOBeHmHw4j1CJTcbfgW0SCFMTXPKHY60BiI/WpZHf9ODwA jol+6HXvzvPGtnZJjemWJAGRRuSIgoHovG/GooEqbLyQH6q2UZ7yVEQItUx/1yzfaBmA 2MCqTW9BudBx3BmJAUtwL3S3lRgF1XsJTkJK/sTEHjjWkHjtJFPevWneAdyX0BvnHaXe XFRolmKWYSZme6PT/xGLItnI3OHgqsgfpB5WdcEYmPKtOtk2pJfAIv8vJqbmCiKMddEg iA== Received: from beta.dmz-eu.st.com (beta.dmz-eu.st.com [164.129.1.35]) by mx07-00178001.pphosted.com (PPS) with ESMTPS id 3rsxjc17c2-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 12 Jul 2023 20:50:28 +0200 Received: from euls16034.sgp.st.com (euls16034.sgp.st.com [10.75.44.20]) by beta.dmz-eu.st.com (STMicroelectronics) with ESMTP id 9FA5610005E for ; Wed, 12 Jul 2023 20:50:27 +0200 (CEST) Received: from Webmail-eu.st.com (shfdag1node3.st.com [10.75.129.71]) by euls16034.sgp.st.com (STMicroelectronics) with ESMTP id 2FC8722FA26 for ; Wed, 12 Jul 2023 20:50:27 +0200 (CEST) Received: from [10.252.3.53] (10.252.3.53) by SHFDAG1NODE3.st.com (10.75.129.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Wed, 12 Jul 2023 20:50:26 +0200 Message-ID: <74633613-8420-5ad6-2882-6ad14066f781@foss.st.com> Date: Wed, 12 Jul 2023 20:50:25 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty Content-Language: en-US To: References: <1de3b3ee-7dd8-db16-6e17-365dbd9fde84@fibranet.cat> From: Torbjorn SVENSSON In-Reply-To: <1de3b3ee-7dd8-db16-6e17-365dbd9fde84@fibranet.cat> Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.252.3.53] X-ClientProxiedBy: EQNCAS1NODE3.st.com (10.75.129.80) To SHFDAG1NODE3.st.com (10.75.129.71) X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.957,Hydra:6.0.591,FMLib:17.11.176.26 definitions=2023-07-12_13,2023-07-11_01,2023-05-22_02 X-Spam-Status: No, score=-10.1 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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