NetBSD Problem Report #43639

From www@NetBSD.org  Sun Jul 18 17:34:22 2010
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id ADD0663BB0A
	for <gnats-bugs@gnats.NetBSD.org>; Sun, 18 Jul 2010 17:34:22 +0000 (UTC)
Message-Id: <20100718173422.64D0F63BAFE@www.NetBSD.org>
Date: Sun, 18 Jul 2010 17:34:22 +0000 (UTC)
From: billycoutsis@gmail.com
Reply-To: billycoutsis@gmail.com
To: gnats-bugs@NetBSD.org
Subject: sh allows user to source a directory
X-Send-Pr-Version: www-1.0

>Number:         43639
>Category:       bin
>Synopsis:       sh allows user to source a directory
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    bin-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Jul 18 17:35:00 +0000 2010
>Closed-Date:    Thu Mar 23 11:37:49 +0000 2017
>Last-Modified:  Thu Mar 23 11:37:49 +0000 2017
>Originator:     Billy Coutsis
>Release:        5.0.2
>Organization:
>Environment:
NetBSD tamper.bmc.local 5.0.2 NetBSD 5.0.2 (GENERIC) #0: Sat Feb  6 17:53:27 UTC 2010  builds@b7.netbsd.org:/home/builds/ab/netbsd-5-0-2-RELEASE/i386/201002061851Z-obj/home/builds/ab/netbsd-5-0-2-RELEASE/src/sys/arch/i386/compile/GENERIC i386
>Description:
sh seems to try and source the literal contents of a filesystem directory entry, instead of failing with an error.

I can't think of a sane reason why a user would want to do this. For comparison, bash on Linux fails with message "is a directory" and exit code 1.
>How-To-Repeat:
1) From a /bin/sh shell, try and source a directory, eg. ". /" or ". /usr"
2) Observe that the shell seems to try and source the literal contents of the filesystem directory entry.
>Fix:
Make the shell fail with an error when the user tries to do this. Of course, any fix should not conflict with any standard that sh should conform to.

>Release-Note:

>Audit-Trail:

Responsible-Changed-From-To: port-i386-maintainer->bin-bug-people
Responsible-Changed-By: dholland@NetBSD.org
Responsible-Changed-When: Fri, 30 Jul 2010 05:51:40 +0000
Responsible-Changed-Why:
not i386-specific


From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/43639 CVS commit: src/bin/sh
Date: Tue, 3 May 2016 03:12:40 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Tue May  3 03:12:40 UTC 2016

 Modified Files:
 	src/bin/sh: eval.c

 Log Message:
 PR bin/43639 - check that a file being read by the '.' command
 is a regular file, even when it is given as a full pathname.


 To generate a diff of this commit:
 cvs rdiff -u -r1.120 -r1.121 src/bin/sh/eval.c

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

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@www.NetBSD.org
Cc: 
Subject: Re: PR/43639 
Date: Wed, 04 May 2016 04:28:21 +0700

 Can you check and see if the sh in NetBSD current does what you
 expect ?

 kre

State-Changed-From-To: open->feedback
State-Changed-By: kre@NetBSD.org
State-Changed-When: Tue, 03 May 2016 21:33:38 +0000
State-Changed-Why:
Waiting upon confirmation that the bug is correcly fixed.


From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/43639 CVS commit: src/bin/sh
Date: Wed, 4 May 2016 08:41:00 +0000

 On Tue, May 03, 2016 at 03:15:00AM +0000, Robert Elz wrote:
  >  Log Message:
  >  PR bin/43639 - check that a file being read by the '.' command
  >  is a regular file, even when it is given as a full pathname.

 out of a general sense of mischief:

 should it be allowed to be a named pipe?

 -- 
 David A. Holland
 dholland@netbsd.org

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/43639 CVS commit: src/bin/sh
Date: Thu, 05 May 2016 00:43:49 +0700

     Date:        Wed,  4 May 2016 08:45:01 +0000 (UTC)
     From:        David Holland <dholland-bugs@netbsd.org>
     Message-ID:  <20160504084501.9E8D87AA9F@mollari.NetBSD.org>

   |   >  PR bin/43639 - check that a file being read by the '.' command
   |   >  is a regular file, even when it is given as a full pathname.
   |  
   |  out of a general sense of mischief:

 Sorry, to get your badge for mischievousness you will need to try harder!

   |  should it be allowed to be a named pipe?

 Perhaps.   I received the same (approximate) message via private mail.
 Personally I also wondered about allowing /dev/null (well!) and perhaps
 more rationally /dev/tty - except that wllowing either of those would
 probably mean allowing all char type devices (I cannot see much utility
 in allowing ". block-dev".)

 For any of this, a PR requesting a change, and saying why it is needed
 would probably be the way to start.

 I fixed the PR (which only actually requested that directories not be
 subject to '.' commands - which along with block devices, are the two
 that are clearly useless) the way I did to make the "pathname given"
 and the "no path in the filename, search $PATH) cases behave the same.
 The "even when" words in the commit message were meant to (at least)
 hint at that.

 That is, from before this fix,
 	PATH=named-pipe-directory:$PATH
 	. pipe-file-name			# no '/' in that name
 would not have worked.

 Now
 	. named-pipe-directory/pipe-file-name
 doesn't either.

 If a change is to be made, should it be made to both cases, or just one
 of them?

 Either way - that info ought be in the (purported) future PR that
 requests this change...

 kre

From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/43639 CVS commit: src/bin/sh
Date: Tue, 31 May 2016 06:26:11 +0000

 On Wed, May 04, 2016 at 05:45:00PM +0000, Robert Elz wrote:
  >    |   >  PR bin/43639 - check that a file being read by the '.' command
  >    |   >  is a regular file, even when it is given as a full pathname.
  >    |  
  >    |  out of a general sense of mischief:
  >  
  >  Sorry, to get your badge for mischievousness you will need to try harder!

 I'll get right on it! :-)

  >    |  should it be allowed to be a named pipe?
  >  
  >  Perhaps. [...]

 Hmm. Well, given this:

  >  That is, from before this fix,
  >  	PATH=named-pipe-directory:$PATH
  >  	. pipe-file-name			# no '/' in that name
  >  would not have worked.
  >  
  >  Now
  >  	. named-pipe-directory/pipe-file-name
  >  doesn't either.

 I'm not convinced it's worth worrying about.

 -- 
 David A. Holland
 dholland@netbsd.org

From: Robert Elz <kre@munnari.OZ.AU>
To: gnats-bugs@NetBSD.org
Cc: 
Subject: Re: PR/43639 CVS commit: src/bin/sh
Date: Tue, 31 May 2016 14:33:34 +0700

     Date:        Tue, 31 May 2016 06:30:00 +0000 (UTC)
     From:        David Holland <dholland-bugs@netbsd.org>
     Message-ID:  <20160531063000.B15027AAC1@mollari.NetBSD.org>

   |  I'm not convinced it's worth worrying about.

 No, for named pipes I wouldn't be either, that is not a very likely use case.
 But Joerg's ". /dev/null" is more likely.  . /dev/tty could happen too I guess.

 So, it may be that only directories (as file types - maybe block devices too?)
 get ignored as candicates for . - and anything unreadable (currently the code
 finds the file, and if it ends up being unreadable, you lose - posix actually
 requires, as I read it, that we keep looking for a file that is readable -
 when searching $PATH of course, the path name case is easy.)

 kre

From: "Robert Elz" <kre@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/43639 CVS commit: src/bin/sh
Date: Wed, 1 Jun 2016 05:10:41 +0000

 Module Name:	src
 Committed By:	kre
 Date:		Wed Jun  1 05:10:41 UTC 2016

 Modified Files:
 	src/bin/sh: eval.c

 Log Message:
 PR bin/43639

 Redo earlier fix to only prohibit sourcing directories and block special files.
 char specials (/dev/tty, /dev/null, ... incl /dev/rwd0a) and fifos are OK.

 Posix actually requires that we find only readable files - that is not yet
 implemented (doing it sanely, without opening the file twice, is going to
 take some more modifications to code elsewhere).


 To generate a diff of this commit:
 cvs rdiff -u -r1.127 -r1.128 src/bin/sh/eval.c

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

State-Changed-From-To: feedback->closed
State-Changed-By: kre@NetBSD.org
State-Changed-When: Thu, 23 Mar 2017 11:37:49 +0000
State-Changed-Why:
Feedback timeout, and it seems as if we have an adequate fix for this now.


>Unformatted:

Home	PR Database Search