From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from omta002.cacentral1.a.cloudfilter.net (omta002.cacentral1.a.cloudfilter.net [3.97.99.33]) by sourceware.org (Postfix) with ESMTPS id 646933858C62 for ; Wed, 12 Jul 2023 20:06:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 646933858C62 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=Shaw.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=shaw.ca Received: from shw-obgw-4004a.ext.cloudfilter.net ([10.228.9.227]) by cmsmtp with ESMTP id JaBYqRI4Q6NwhJg70q7Rv4; Wed, 12 Jul 2023 20:06:58 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=shaw.ca; s=s20180605; t=1689192418; bh=qNVmZLYSf5HdXFgqlaghH4oFNpUTDL12aYq28xWzjJE=; h=Date:Reply-To:Subject:To:References:From:In-Reply-To; b=tUj6M2sr48WsbCs4U5Aw0v99k8GKu3MgnBvLHj+OR/BinI0k/FfAIO65HOjnBzybG gekOqzCSvM60rr6qo3QQQGD/co0x493WBQwPAQMdkcYolq0ai4zom1tbjs9nQUFA/w srPvTmQ3futvnYCm9ERnjc2TbSRBDyMmeYGRHqpbRVky1boA16NMnHmTyQ805goXmS YmTWau9F8wFifJkgpQUmE/g8HjQzil2zCmI+EXLY1pvEV9ZHyW6v/IASOOsr/xocxd ZIKgM4MDfk4MXZ37MFQY8hqNynZ6gnkMYPlh1xltMJBvJyGKUTyKZfRDGPExUyX25w SebY49dsA+b2A== Received: from [10.0.0.5] ([184.64.102.149]) by cmsmtp with ESMTP id Jg6zqDA2b3fOSJg70qt4oE; Wed, 12 Jul 2023 20:06:58 +0000 X-Authority-Analysis: v=2.4 cv=J8G5USrS c=1 sm=1 tr=0 ts=64af07e2 a=DxHlV3/gbUaP7LOF0QAmaA==:117 a=DxHlV3/gbUaP7LOF0QAmaA==:17 a=IkcTkHD0fZMA:10 a=eFtDlIn6AAAA:8 a=CCpqsmhAAAAA:8 a=yuJ0Uhft01uisukBEcoA:9 a=QEXdDO2ut3YA:10 a=OsY-XHaDHCpehLR_XMTN:22 a=ul9cdbp4aOFLsgKbc677:22 Message-ID: Date: Wed, 12 Jul 2023 14:06:57 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Reply-To: newlib@sourceware.org Subject: Re: [PATCH] Fix getlogin() to check only stdin to get a valid tty Content-Language: en-CA To: newlib@sourceware.org References: <1de3b3ee-7dd8-db16-6e17-365dbd9fde84@fibranet.cat> <74633613-8420-5ad6-2882-6ad14066f781@foss.st.com> From: Brian Inglis Organization: Inglis In-Reply-To: <74633613-8420-5ad6-2882-6ad14066f781@foss.st.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CMAE-Envelope: MS4xfP8iSUiUSk6WtgPBbiMVUrJOtvmWR/icg7UCjLMsj5rKOVS/jiyeB9c7fcKbRIA5X21SUBzv4JsB+48Hg+9Gaa3Tq9bSuLr0OiVJv4LF0bd1EK1BDGOa MDQwdMkgCj1++tfBhwnlfbkMJDZaycbM3tdFB/VrK/3m5k/UQHFl3XBNLhha8e14DFNmB9WAewxF4Q== X-Spam-Status: No, score=-7.9 required=5.0 tests=BAYES_00,BODY_8BITS,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,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 12:50, Torbjorn SVENSSON wrote: > > > 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. No worries: Cygwin getlogin/_r is defined as "C" in winsup/cygwin/uinfo.cc to deal with Windows and limits itself to UNLEN: /usr/include/cygwin/limits.h:#define __LOGIN_NAME_MAX 256 /* equal to UNLEN defined in w32api/lmcons.h */ /usr/include/w32api/lmcons.h:#define UNLEN 256 -- Take care. Thanks, Brian Inglis Calgary, Alberta, Canada La perfection est atteinte Perfection is achieved non pas lorsqu'il n'y a plus rien à ajouter not when there is no more to add mais lorsqu'il n'y a plus rien à retirer but when there is no more to cut -- Antoine de Saint-Exupéry