Re: CDPATH and shell scripts

[Goswin von Brederlow]

Mike Hommey  writes:

> On Thu, Jul 02, 2009 at 02:26:21PM -0700, Russ Allbery wrote:
>> Jonathan Yu  writes:
>> 
>> > How to fix them? Write Perl scripts, and turn on taint checking --
>> > that fixes the four issues above, because it makes the script exit if
>> > any of them look dangerous. Env::Sanctify::Auto is a Perl module that
>> > automatically cleans up the paths.
>> >
>> > My advice:
>> > 1. Write scripts that might be run as root (or setuid root) using Perl
>> > 2. Turn on taint checking
>> > 3. Consider using Env::Sanctify::Auto (shameless plug)
>> 
>> I would really prefer that people not start writing maintainer scripts
>> in Perl as a matter of course.  Perl is harder to analyze for programs
>> like lintian than shell scripts (which are already hard enough).
>
> I wonder, do dpkg unset these variables when running maintainer scripts?
> That could be a good idea if it doesn't already.
>
> Mike

It does not, at least not specifically. Nor do nearly all shell
scripts in /usr/bin.

And think of what fun that would be to debug for a debconf using
package. Suddenly debconf gets told some paths and errors out.


MfG
Goswin


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Mike Hommey]

On Thu, Jul 02, 2009 at 02:26:21PM -0700, Russ Allbery wrote:
> Jonathan Yu  writes:
> 
> > How to fix them? Write Perl scripts, and turn on taint checking --
> > that fixes the four issues above, because it makes the script exit if
> > any of them look dangerous. Env::Sanctify::Auto is a Perl module that
> > automatically cleans up the paths.
> >
> > My advice:
> > 1. Write scripts that might be run as root (or setuid root) using Perl
> > 2. Turn on taint checking
> > 3. Consider using Env::Sanctify::Auto (shameless plug)
> 
> I would really prefer that people not start writing maintainer scripts
> in Perl as a matter of course.  Perl is harder to analyze for programs
> like lintian than shell scripts (which are already hard enough).

I wonder, do dpkg unset these variables when running maintainer scripts?
That could be a good idea if it doesn't already.

Mike


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[brian m. carlson]

On Fri, Jul 03, 2009 at 01:01:41AM +0200, Goswin von Brederlow wrote:
> As a middle ground I wouldn't mind $SHELL to unset CDPATH when it
> switches from an interactive shell to a non-interactive shell, when a
> script with #! $SHELL is executed. That one is just to damn scary.

I don't think that's permitted by SUSv3, and Policy says:

  Scripts may assume that /bin/sh implements the SUSv3 Shell Command
  Language

> Also why does it output to stdout and not stderr?

According to SUSv3 (and SUSv4) for the cd utility:

  If a non-empty directory name from CDPATH is used, or if cd - is used,
  an absolute pathname of the new working directory shall be written to
  the standard output as follows:
  
  "%s\n", 

The rationale states that CDPATH was taken from the SysV shell, and it
probably had that behavior.

If you're going to write a /bin/sh script, you need to be aware of the
environment and how it's going to affect your script.  For example, if
you don't want utilities (such as patch) to have POSIX behavior, you
need to unset POSIXLY_CORRECT and _POSIX2_VERSION.  If you don't want
cd to use CDPATH, you need to unset it.  You can probably do something
like

  unset `set | sed -re 's/^([^=]+)=.*$/\1/g' | grep -E '^[A-Za-z0-9]' | grep 
-vE '^(PS|PATH|TERM)'` 2>/dev/null

to get a "clean" environment.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 713 440 7475 | http://crustytoothpaste.ath.cx/~bmc | My opinion only
OpenPGP: RSA v4 4096b 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187


signature.asc
Description: Digital signature

Re: CDPATH and shell scripts

[Goswin von Brederlow]

Jonathan Yu  writes:

> Another option might be to break from POSIX/etc policy (I'm not sure
> where these variables are defined) and patch our command like 'cd' to
> simply ignore 'CDPATH' etc. But I suppose this would then require
> patches in all the various shells available for Debian to go against
> something standardized for so long.
>
> It's a contentious issue. I wish everyone all the best trying to
> figure it out, it's scary stuff indeed.
>
> Cheers,
>
> Jonathan

As a middle ground I wouldn't mind $SHELL to unset CDPATH when it
switches from an interactive shell to a non-interactive shell, when a
script with #! $SHELL is executed. That one is just to damn scary.

Also why does it output to stdout and not stderr?

MfG
Goswin


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Russ Allbery]

Jonathan Yu  writes:
> On Thu, Jul 2, 2009 at 5:26 PM, Russ Allbery wrote:

>> I would really prefer that people not start writing maintainer
>> scripts in Perl as a matter of course.  Perl is harder to analyze for
>> programs like lintian than shell scripts (which are already hard
>> enough).

> I agree that all too often many build systems are sort of centered
> around Perl, and that can be a bad thing, in terms of maintainability.
> On the other hand I don't think Perl is always unmaintainable -- it
> depends on the programmer.

Just to clarify, it's really not maintainability that I'm concerned
about.  It's automated analysis.  Take a look at checks/scripts,
checks/menus, checks/debconf, and checks/shared-libs in Lintian, among
others.  Currently, we mostly just bail if Perl is used for maintainer
scripts since it's much more difficult to do the sort of checks that
Lintian is doing against Perl scripts.  There are more ways of
expressing conditionals that are widely used, there's additional
punctuation and potential line wrapping in system() calls, and all sorts
of other issues.

Perl is a problem specifically because it's a richer language.  Any
other richer language would also be a problem.  For better or for ill,
Bourne shell is fairly straightforward, and while it has some odd
corners, it doesn't have that many syntactic quirks or that many
different ways of doing the same thing.  It also helps considerably that
the way to run an external program (which is much of what Lintian
checks) is to just run it, without additional syntax or function calls.

Note also that if you use Perl to write maintainer scripts but you're
using debhelper, you have to use somewhat strange workarounds to get the
debhelper fragments to be invoked, and those too are hard to analyze.

I've been quixotically proposing for several years that the ideal
solution would be to write a new interpreter specifically for maintainer
scripts that had a simple syntax and directly supported the 20-30 most
common things that one needs to do in a Debian maintainer script.  One
could start with anything that debhelper adds to maintainer scripts and
then add diversions, alternatives, deleting files and directories, and
maybe two or three other things.  Packages that need something more
complicated could fall back to shell or Perl as they do today, but I
suspect that writing an interpreter that could handle the maintainer
scripts for 90% of the packages in Debian would not be particularly
difficult, and it would also potentially hide a lot of complexity and
would be an opportunity to get the behavior right in all the tricky
error rollback cases.

Of course, I have no time to do the work.  :/

-- 
Russ Allbery (r...@debian.org)   


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Jonathan Yu]

On Thu, Jul 2, 2009 at 5:44 PM, Goswin von Brederlow wrote:
> Russ Allbery  writes:
>
>> Jonathan Yu  writes:
>>
>>> How to fix them? Write Perl scripts, and turn on taint checking --
>>> that fixes the four issues above, because it makes the script exit if
>>> any of them look dangerous. Env::Sanctify::Auto is a Perl module that
>>> automatically cleans up the paths.
>>>
>>> My advice:
>>> 1. Write scripts that might be run as root (or setuid root) using Perl
>>> 2. Turn on taint checking
>>> 3. Consider using Env::Sanctify::Auto (shameless plug)
>>
>> I would really prefer that people not start writing maintainer scripts
>> in Perl as a matter of course.  Perl is harder to analyze for programs
>> like lintian than shell scripts (which are already hard enough).
>>
>> --
>> Russ Allbery (r...@debian.org)               
>
> Not to mention humans. :)
Oh.

I read that incorrectly in my reply. Yeah, analyzing Perl via lintian
is probably more difficult than shell script, and more likely to raise
false positives/etc.

Static code analysis is getting a little better with PPI, but Perl is
a *very* dynamic language, so it's hard to do analyze it without
executing it.

In that case, I'd recommend to establish some sort of best practice
using these variables, and put it in Policy. As in a contract that
programs:
1. Must not set these variables
2. Must not use these variables
3. Must set them to something sane within the scope of the current
execution (not necessarily exporting them)

So I guess really in practice that just requires people to do:
unset CDPATH
unset IFS
set PATH="/some/safe/path'

etc. All of these default/preferred values should be in Policy, and
users should be given a list of acceptable things to go in PATH -- for
example, allowing developers to use:
/usr/bin
/usr/local/bin

Then a developer is welcome to locally set PATH to a subset of those,
like PATH=/usr/bin -- of course we'd need to add something to lintian
to parse paths. There are, however, CPAN modules that can do that I
believe, and on Debian only it's as simple as split(':', $path)


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Goswin von Brederlow]

Russ Allbery  writes:

> Jonathan Yu  writes:
>
>> How to fix them? Write Perl scripts, and turn on taint checking --
>> that fixes the four issues above, because it makes the script exit if
>> any of them look dangerous. Env::Sanctify::Auto is a Perl module that
>> automatically cleans up the paths.
>>
>> My advice:
>> 1. Write scripts that might be run as root (or setuid root) using Perl
>> 2. Turn on taint checking
>> 3. Consider using Env::Sanctify::Auto (shameless plug)
>
> I would really prefer that people not start writing maintainer scripts
> in Perl as a matter of course.  Perl is harder to analyze for programs
> like lintian than shell scripts (which are already hard enough).
>
> -- 
> Russ Allbery (r...@debian.org)   

Not to mention humans. :)

MfG
Goswin


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Jonathan Yu]

On Thu, Jul 2, 2009 at 5:26 PM, Russ Allbery wrote:
> Jonathan Yu  writes:
>
>> How to fix them? Write Perl scripts, and turn on taint checking --
>> that fixes the four issues above, because it makes the script exit if
>> any of them look dangerous. Env::Sanctify::Auto is a Perl module that
>> automatically cleans up the paths.
>>
>> My advice:
>> 1. Write scripts that might be run as root (or setuid root) using Perl
>> 2. Turn on taint checking
>> 3. Consider using Env::Sanctify::Auto (shameless plug)
>
> I would really prefer that people not start writing maintainer scripts
> in Perl as a matter of course.  Perl is harder to analyze for programs
> like lintian than shell scripts (which are already hard enough).

I agree that all too often many build systems are sort of centered
around Perl, and that can be a bad thing, in terms of maintainability.
On the other hand I don't think Perl is always unmaintainable -- it
depends on the programmer.

I suppose though that shell scripts are simpler and thus present less
of a cognitive load for programmers, so that might be why it's
perceived to be more maintainable/easier to analyze.

On the other hand, you're going to run into security issues due to the
aforementioned variables, in any language that doesn't provide a check
(like Perl's taint mode).

Perhaps it could be established in Policy as a best practice to set
those environment variables at the beginning of execution, and carry
them throughout. Or to use a simple Perl or Shell script wrapper which
would clear all those variables before exec'ing to the desired
command.

Security versus convenience is a common debate, and it's no different here.

Another option might be to break from POSIX/etc policy (I'm not sure
where these variables are defined) and patch our command like 'cd' to
simply ignore 'CDPATH' etc. But I suppose this would then require
patches in all the various shells available for Debian to go against
something standardized for so long.

It's a contentious issue. I wish everyone all the best trying to
figure it out, it's scary stuff indeed.

Cheers,

Jonathan


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Russ Allbery]

Jonathan Yu  writes:

> How to fix them? Write Perl scripts, and turn on taint checking --
> that fixes the four issues above, because it makes the script exit if
> any of them look dangerous. Env::Sanctify::Auto is a Perl module that
> automatically cleans up the paths.
>
> My advice:
> 1. Write scripts that might be run as root (or setuid root) using Perl
> 2. Turn on taint checking
> 3. Consider using Env::Sanctify::Auto (shameless plug)

I would really prefer that people not start writing maintainer scripts
in Perl as a matter of course.  Perl is harder to analyze for programs
like lintian than shell scripts (which are already hard enough).

-- 
Russ Allbery (r...@debian.org)   


-- 
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Jonathan Yu]

Hi:

There are lots of variables which do nasty things.

In particular (copying this from perldoc of a module I wrote):
PATH

PATH provides a list of paths to search for executables, which
influences which commands are invoked by unqualified calls to system()
and others. This variable is particularly dangerous because even if
you use a fully qualified call to the executable, like "/usr/bin/echo
..." -- there is still a security hole, since echo could be executing
unqualified code itself.

The safest way to handle this, and the strategy used by this module,
is to remove everything except /usr/bin and /usr/local/bin (or
equivalent, depending on your operating system).
CDPATH

CDPATH provides additional paths for cd to search on the system when
it is called. This is dangerous because you could be attempting to
change into a known safe directory, but the CDPATH may divert you to
another directory. The variable is generally of limited usefulness,
and so is removed completely during %ENV scrubbing.
IFS

IFS is the Internal Field Separator, which tells the operating system
what characters should be considered whitespace separating command
line arguments. Combined with controlling PATH, this exposes a very
dangerous vulnerability: if the IFS is set to '/', then
system('/bin/more') is essentially the same as system('bin more'). As
a result, the 'bin' command is executed instead of '/bin/more' as
expected.
ENV and BASH_ENV

ENV and BASH_ENV list files that are executed whenever a new shell is
started, which includes whenever a shell script (.sh) is run.

Okay. So what? These issues have existed for a long time.

How to fix them? Write Perl scripts, and turn on taint checking --
that fixes the four issues above, because it makes the script exit if
any of them look dangerous. Env::Sanctify::Auto is a Perl module that
automatically cleans up the paths.

My advice:
1. Write scripts that might be run as root (or setuid root) using Perl
2. Turn on taint checking
3. Consider using Env::Sanctify::Auto (shameless plug)

Cheers,

Jonathan

On Thu, Jul 2, 2009 at 4:22 PM, Michael Tautschnig wrote:
> [...]
>> So what is the right course of action here?
>>
>> 1) unset CDPATH in every single shell script there is?
>> 2) never use relartive paths for cd in scripts?
>> 3) shoot the user for doing something dumb?
>> 4) disable CDPATH in /bin/sh (or is that POSIX?) or non-interactive
>>    scripts (would break automake I think)
>>
>
> Looking at some autoconf-generated configure script I've found the following:
>
> 
> # The HP-UX ksh and POSIX shell print the target directory to stdout
> # if CDPATH is set.
> (unset CDPATH) >/dev/null 2>&1 && unset CDPATH
> 
>
> I had once stumbled over this problem and ever since I keep using
>
> cd bla > /dev/null
>
> whenever the output could possibly trouble me. But well, actually, unsetting
> CDPATH would be more appropriate.
>
> Best,
> Michael
>
>


--
To UNSUBSCRIBE, email to debian-devel-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org

Re: CDPATH and shell scripts

[Michael Tautschnig]

[...]
> So what is the right course of action here?
> 
> 1) unset CDPATH in every single shell script there is?
> 2) never use relartive paths for cd in scripts?
> 3) shoot the user for doing something dumb?
> 4) disable CDPATH in /bin/sh (or is that POSIX?) or non-interactive
>scripts (would break automake I think)
> 

Looking at some autoconf-generated configure script I've found the following:


# The HP-UX ksh and POSIX shell print the target directory to stdout
# if CDPATH is set.
(unset CDPATH) >/dev/null 2>&1 && unset CDPATH


I had once stumbled over this problem and ever since I keep using

cd bla > /dev/null

whenever the output could possibly trouble me. But well, actually, unsetting
CDPATH would be more appropriate.

Best,
Michael



pgpbQZmW16g4o.pgp
Description: PGP signature