Eric Blake | 8 Feb 2012 18:27
Picon
Favicon
Gravatar

bug in handling of ignored traps

Per POSIX requirements on trap, dash is properly refusing to let
non-interactive scripts reset signal handlers via trap if the shell was
started with an inherited ignored signal handler.  However, dash lies to
the user, making it impossible to tell if the user is invoking a shell
in an environment where a signal was inherited as ignored.  ksh and bash
are nicer about things, and at least let the user query whether a shell
is treating a particular signal as non-resettable.

This is important for shell scripts that WANT to guarantee a particular
behavior of SIGPIPE handling (such as this thread on writing a grep test
for covering the behavior of grep both with and without SIGPIPE
inherited as ignored:
https://lists.gnu.org/archive/html/bug-grep/2012-02/msg00016.html).

Bash behavior - you can learn about the ignored handler from the get-go:

$ (trap '' PIPE; bash -c 'trap; echo 0; trap "echo 1" PIPE; trap; \
     echo 2; kill -s PIPE $$; trap; echo 3')
trap -- '' SIGPIPE
0
trap -- '' SIGPIPE
2
trap -- '' SIGPIPE
3

Ksh behavior - you can learn about the ignored handler, but only after
trying to use something else:

$ (trap '' PIPE; ksh -c 'trap; echo 0; trap "echo 1" PIPE; trap; \
     echo 2; kill -s PIPE $$; trap; echo 3')
(Continue reading)

Chet Ramey | 8 Feb 2012 21:12
Favicon

Re: bug in handling of ignored traps

> Per POSIX requirements on trap, dash is properly refusing to let
> non-interactive scripts reset signal handlers via trap if the shell was
> started with an inherited ignored signal handler.  However, dash lies to
> the user, making it impossible to tell if the user is invoking a shell
> in an environment where a signal was inherited as ignored.  ksh and bash
> are nicer about things, and at least let the user query whether a shell
> is treating a particular signal as non-resettable.

I don't think this is a bug.  It's more of a quality-of-implementation
issue.  dash appears to be following the "no error need be reported
when attempting to do so" part of the Posix spec, which is fine.  Bash
did more or less the same thing until bash-4.2.

Chet

--

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
		 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, ITS, CWRU    chet <at> case.edu    http://cnswww.cns.cwru.edu/~chet/
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

harald | 14 Feb 2012 11:48
Picon
Favicon
Gravatar

[PATCH] var.c: check for valid variable name before printing in "export -p"

From: Harald Hoyer <harald <at> redhat.com>

"export -p" prints all environment variables, without checking if the
environment variable is a valid dash variable name.

IMHO, the only valid usecase for "export -p" is to eval the output.

$ eval $(export -p); echo OK
OK

Without this patch the following test does error out with:

test.py:
import os
os.environ["test-test"]="test"
os.environ["test_test"]="test"
os.execv("./dash", [ './dash', '-c', 'eval $(export -p); echo OK' ])

$ python test.py
./dash: 1: export: test-test: bad variable name

Of course the results can be more evil, if the environment variable
name is crafted, that it injects valid shell code.
---
 src/var.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/var.c b/src/var.c
index 027beff..06771d3 100644
--- a/src/var.c
(Continue reading)

Herbert Xu | 25 Feb 2012 08:36
Picon
Picon

Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

On Tue, Feb 14, 2012 at 10:48:48AM +0000, harald <at> redhat.com wrote:
> From: Harald Hoyer <harald <at> redhat.com>
> 
> "export -p" prints all environment variables, without checking if the
> environment variable is a valid dash variable name.
> 
> IMHO, the only valid usecase for "export -p" is to eval the output.

Thanks a lot for the report and patch.

I'd prefer to fix this up at entry into the shell rather than
when we print out the variables.  So how about this patch?

commit 46d3c1a614f11f0d40a7e73376359618ff07abcd
Author: Herbert Xu <herbert <at> gondor.apana.org.au>
Date:   Sat Feb 25 15:35:18 2012 +0800

    [VAR] Sanitise environment variable names on entry

    On Tue, Feb 14, 2012 at 10:48:48AM +0000, harald <at> redhat.com wrote:
    >
    > "export -p" prints all environment variables, without checking if the
    > environment variable is a valid dash variable name.
    >
    > IMHO, the only valid usecase for "export -p" is to eval the output.
    >
    > $ eval $(export -p); echo OK
    > OK
    >
    > Without this patch the following test does error out with:
(Continue reading)

Herbert Xu | 25 Feb 2012 15:31
Picon
Picon

Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

On Sat, Feb 25, 2012 at 03:30:04PM +0100, Jilles Tjoelker wrote:
>
> Most shells pass the environment variable through, such as bash, zsh,
> ksh93 and most ash derivatives. However, the original Bourne shell and
> pdksh/mksh do not.

Do you know of any genuine uses of such environment variables?

Cheers,
--

-- 
Email: Herbert Xu <herbert <at> gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jilles Tjoelker | 25 Feb 2012 15:30
Picon
Favicon

Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

On Sat, Feb 25, 2012 at 06:36:36PM +1100, Herbert Xu wrote:
> On Tue, Feb 14, 2012 at 10:48:48AM +0000, harald <at> redhat.com wrote:
> > From: Harald Hoyer <harald <at> redhat.com>

> > "export -p" prints all environment variables, without checking if the
> > environment variable is a valid dash variable name.

> > IMHO, the only valid usecase for "export -p" is to eval the output.

> Thanks a lot for the report and patch.

> I'd prefer to fix this up at entry into the shell rather than
> when we print out the variables.  So how about this patch?

Such a change would change other things than just "set" and "export -p".
It would also not propagate environment variables with invalid names to
child processes. For example:

env -i not-a-name=1 PATH="$PATH" dash -c env | grep not-a-name

Most shells pass the environment variable through, such as bash, zsh,
ksh93 and most ash derivatives. However, the original Bourne shell and
pdksh/mksh do not.

I think it is best to pass them through so that executing a utility with
and without the shell are as similar as possible (most versions of
make(1) assume this is the case by skipping the shell if the command is
simple enough) and particularly for dash for historical/compatibility
reasons.

(Continue reading)

Eric Blake | 25 Feb 2012 15:53
Picon
Favicon
Gravatar

Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

On 02/25/2012 07:31 AM, Herbert Xu wrote:
> On Sat, Feb 25, 2012 at 03:30:04PM +0100, Jilles Tjoelker wrote:
>>
>> Most shells pass the environment variable through, such as bash, zsh,
>> ksh93 and most ash derivatives. However, the original Bourne shell and
>> pdksh/mksh do not.
> 
> Do you know of any genuine uses of such environment variables?

POSIX states that applications must not rely on such pass-through:
http://austingroupbugs.net/view.php?id=168

So while it might indeed be useful to pass through invalid names, such
an application is broken for expecting it to work, and I'm okay with
this patch as-is.

--

-- 
Eric Blake   eblake <at> redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Herbert Xu | 25 Feb 2012 15:54
Picon
Picon

Re: [PATCH] var.c: check for valid variable name before printing in "export -p"

On Sat, Feb 25, 2012 at 07:53:24AM -0700, Eric Blake wrote:
>
> POSIX states that applications must not rely on such pass-through:
> http://austingroupbugs.net/view.php?id=168
> 
> So while it might indeed be useful to pass through invalid names, such
> an application is broken for expecting it to work, and I'm okay with
> this patch as-is.

Great.  From a security point of view I'd rather not pass them
through.

Cheers,
--

-- 
Email: Herbert Xu <herbert <at> gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe dash" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Gmane