NetBSD Problem Report #56464

From elo@foobox.net  Mon Oct 25 04:58:04 2021
Return-Path: <elo@foobox.net>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 4C6221A9239
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 25 Oct 2021 04:58:04 +0000 (UTC)
Message-Id: <20211025031831.9A2518252@marmite.localnet>
Date: Mon, 25 Oct 2021 04:18:31 +0100 (BST)
From: elo@foobox.net
Reply-To: elo@foobox.net
To: gnats-bugs@NetBSD.org
Subject: sh(1) reads ./.profile rather than ~/.profile
X-Send-Pr-Version: 3.95

>Number:         56464
>Category:       bin
>Synopsis:       sh(1) reads ./.profile rather than ~/.profile
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    kre
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Oct 25 05:00:00 +0000 2021
>Closed-Date:    Sat Nov 06 14:27:15 +0000 2021
>Last-Modified:  Sat Nov 06 14:27:15 +0000 2021
>Originator:     elo
>Release:        NetBSD 9.0_RC1
>Organization:
>Environment:
System: NetBSD marmite.localnet 9.0_RC1 NetBSD 9.0_RC1 (BLUEBELL) #0: Tue Dec 24 17:18:23 GMT 2019  elo@marmite.localnet:/usr/obj/sys/arch/amd64/compile/BLUEBELL amd64
Architecture: x86_64
Machine: amd64
>Description:
	The sh(1) man page states that a login shell reads commands from
	the files /etc/profile and ~/.profile. In reality, sh looks for
	.profile in the current working directory, and not in the user's
	home directory. Commands like login(1) and sudo(8) typically set
	the cwd to the user's $HOME, removing the distinction in common
	use, but '~' and '.' may not coincide for shells started by other
	means, e.g., via su(1) with the '-d' option.

	Revision 1.212 of the sh man page added a qualifying clause (not
	mentioned in the log message for that commit) to say that .profile
	is looked for specifically in the user's home directory (not in
	the cwd, as was before implicit), but the code was never changed
	to implement that behaviour. Every Bourne-like shell I've checked
	(including dash, another Almquist descendant) looks for .profile
	in the home directory of the user.
>How-To-Repeat:
	$ cd /tmp
	$ echo 'echo user:$USER home:$HOME cwd:$PWD' > .profile
	$ su -d <user>

	The choice of <user> is immaterial (save for the obviously
	necessary condition that <user>'s login shell must be /bin/sh).
>Fix:
	Reconcile the sh code and the sh man page, either by removing the
	misleading text added in revision 1.212 of the man page, or by
	amending the code so that the shell really does look for .profile
	in the user's home directory (or in '/', if $HOME is not set).

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: bin-bug-people->kre
Responsible-Changed-By: kre@NetBSD.org
Responsible-Changed-When: Mon, 25 Oct 2021 10:15:06 +0000
Responsible-Changed-Why:
I am looking into this PR


State-Changed-From-To: open->needs-pullups
State-Changed-By: kre@NetBSD.org
State-Changed-When: Tue, 26 Oct 2021 00:07:17 +0000
State-Changed-Why:
Needs a pullup to -9 (after it has settled in a bit).


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56464 CVS commit: src/bin/sh
Date: Tue, 26 Oct 2021 00:05:38 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Tue Oct 26 00:05:38 UTC 2021

 Modified Files:
 	src/bin/sh: main.c memalloc.c memalloc.h options.c sh.1 shell.h

 Log Message:
 PR bin/56464

 After almost 30 years, finally do the right thing and read $HOME/.profile
 rather than .profile in the initial directory (it was that way in version
 1.1 ...)   All other ash descendants seem to have fixed this long ago.

 While here, copy a feature from FreeBSD which allows "set +p" (if a
 shell run by a setuid process with the -p flag is privileged) to reset
 the privileges.  Once done (the set +p) it cannot be undone (a later
 set -p sets the 'p' flag, but that's all it does) - that just becomes a
 one bit storage location.

 We do this, as (also copying from FreeBSD, and because it is the right
 thing to do) we don't run .profile in a privileged shell - FreeBSD run
 /etc/suid_profile in that case (not a good name, it also applies to setgid
 shells) but I see no real need for that, we run /etc/profile in any case,
 anything that would go in /etc/suid_profile can just go in /etc/profile
 instead (with suitable guards so the commands only run in priv'd shells).

 One or two minor DEBUG mode changes (notably having priv'd shells identify
 themselves in the DEBUG trace) and sh.1 changes with doc of the "set +p"
 change, the effect that has on $PSc and a few other wording tweaks.

 XXX pullup -9   (not -8, this isn't worth it for the short lifetime
 that has left - if it took 28+ years for anyone to notice this, it
 cannot be having all that much effect).


 To generate a diff of this commit:
 cvs rdiff -u -r1.86 -r1.87 src/bin/sh/main.c
 cvs rdiff -u -r1.33 -r1.34 src/bin/sh/memalloc.c
 cvs rdiff -u -r1.18 -r1.19 src/bin/sh/memalloc.h
 cvs rdiff -u -r1.55 -r1.56 src/bin/sh/options.c
 cvs rdiff -u -r1.234 -r1.235 src/bin/sh/sh.1
 cvs rdiff -u -r1.30 -r1.31 src/bin/sh/shell.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: needs-pullups->pending-pullups
State-Changed-By: kre@NetBSD.org
State-Changed-When: Fri, 05 Nov 2021 17:41:19 +0000
State-Changed-Why:
pullup-9 #1371 requested.


From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/56464 CVS commit: [netbsd-9] src/bin/sh
Date: Sat, 6 Nov 2021 13:35:43 +0000

 Module Name:	src
 Committed By:	martin
 Date:		Sat Nov  6 13:35:43 UTC 2021

 Modified Files:
 	src/bin/sh [netbsd-9]: main.c memalloc.c memalloc.h options.c sh.1
 	    shell.h

 Log Message:
 Pull up following revision(s) (requested by kre in ticket #1371):

 	bin/sh/main.c: revision 1.87
 	bin/sh/main.c: revision 1.88
 	bin/sh/memalloc.h: revision 1.20
 	bin/sh/sh.1: revision 1.235
 	bin/sh/memalloc.c: revision 1.34
 	bin/sh/memalloc.c: revision 1.35
 	bin/sh/memalloc.h: revision 1.19
 	bin/sh/shell.h: revision 1.31
 	bin/sh/options.c: revision 1.56

 PR bin/56464

 After almost 30 years, finally do the right thing and read $HOME/.profile
 rather than .profile in the initial directory (it was that way in version
 1.1 ...)   All other ash descendants seem to have fixed this long ago.
 While here, copy a feature from FreeBSD which allows "set +p" (if a
 shell run by a setuid process with the -p flag is privileged) to reset
 the privileges.  Once done (the set +p) it cannot be undone (a later
 set -p sets the 'p' flag, but that's all it does) - that just becomes a
 one bit storage location.

 We do this, as (also copying from FreeBSD, and because it is the right
 thing to do) we don't run .profile in a privileged shell - FreeBSD run
 /etc/suid_profile in that case (not a good name, it also applies to setgid
 shells) but I see no real need for that, we run /etc/profile in any case,
 anything that would go in /etc/suid_profile can just go in /etc/profile
 instead (with suitable guards so the commands only run in priv'd shells).

 One or two minor DEBUG mode changes (notably having priv'd shells identify
 themselves in the DEBUG trace) and sh.1 changes with doc of the "set +p"
 change, the effect that has on $PSc and a few other wording tweaks.

 XXX pullup -9   (not -8, this isn't worth it for the short lifetime
 that has left - if it took 28+ years for anyone to notice this, it
 cannot be having all that much effect).

 Use a type-correct end marker for strstrcat() rather than NULL, as
 for a function with unknown number & types of args, the compiler isn't
 able to automatically convert to the correct type.   Issue pointed out
 in off list e-mail by Rolland Illig ... Thanks.

 The first arg (pointer to where to put length of result) is of a known
 type, so doesn't have the same issue - we can keep using NULL for that
 one when the length isn't needed.
 Also, make sure to return a correctly null terminated null string in
 the (absurd) case that there are no non-null args to strstrcat() (though
 there are much better ways to generate "" on the stack).  Since there is
 currently just one call in the code, and it has real string args, this
 isn't an issue for now, but who knows, some day.

 NFCI - if there is any real change, then it is a change that is required.

 XXX pullup -9 (together with the previous changes)


 To generate a diff of this commit:
 cvs rdiff -u -r1.82.2.1 -r1.82.2.2 src/bin/sh/main.c
 cvs rdiff -u -r1.33 -r1.33.2.1 src/bin/sh/memalloc.c
 cvs rdiff -u -r1.18 -r1.18.2.1 src/bin/sh/memalloc.h
 cvs rdiff -u -r1.53 -r1.53.2.1 src/bin/sh/options.c
 cvs rdiff -u -r1.223 -r1.223.2.1 src/bin/sh/sh.1
 cvs rdiff -u -r1.29 -r1.29.2.1 src/bin/sh/shell.h

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: pending-pullups->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Sat, 06 Nov 2021 14:27:15 +0000
State-Changed-Why:
Pullup completed.  This issue is fixed now.


>Unformatted:

Home	PR Database Search