Date: Mon, 16 Jul 2007 07:02:28 -0700
From: "Wes Peters" <barnaclewes@gmail.com>
To: "Andrey Chernov" <ache@nagual.pp.ru>, "Sean C. Farley" <scf@freebsd.org>, "Robert Watson" <rwatson@freebsd.org>, freebsd-current <freebsd-current@freebsd.org>, "Michal Mertl" <mime@traveller.cz>
Subject: Re: Environment handling broken in /bin/sh with changes to {get, set, put}env()
Message-ID: <f83770800707160702g4cd08912pac63753681c7217e@mail.gmail.com>
In-Reply-To: <20070713162742.GA16260@nagual.pp.ru>
References: <20070704215154.O77978@thor.farley.org> <20070705105922.F98700@thor.farley.org> <20070707130859.GA96605@nagual.pp.ru> <20070707131359.GB96605@nagual.pp.ru> <20070707133102.C14065@thor.farley.org> <20070707191835.GA4368@nagual.pp.ru> <20070707205410.B14065@thor.farley.org> <20070708020940.GA80166@nagual.pp.ru> <20070708171727.GA90490@nagual.pp.ru> <20070713162742.GA16260@nagual.pp.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
On 7/13/07, Andrey Chernov <ache@nagual.pp.ru> wrote:
> On Sun, Jul 08, 2007 at 09:17:27PM +0400, Andrey Chernov wrote:
> > Hmm. I just think a bit more and feel worry about that place in the merge
> > code:
> >
> > *equals = '\0';
> > if (setenv(*env, equals + 1, 1) == -1)
> > return (-1);
> > *equals = '=';
> > because it modifies memory which may be treated like const one.
> >
> > Consider following scenario: getenv() is not thread-safe, but may be
> [snip]
>
> I found another breakage case not covered by your last getenv() fix.
> Take this simple program:
>
> -- a.c -------------------------------------------------------------------
> #include <stdlib.h>
> extern char **environ;
>
> main () {
>
> static char *nenv[2];
>
> nenv[0] = "PATH=/bin";
> nenv[1] = NULL;
>
> /*
> environ = nenv;
> unsetenv("PATH"); or somethig like
> which touch '=' char in nenv[0]
> */
>
> nenv[0][4] = '\0';
>
> }
> -- a.c -------------------------------------------------------------------
>
> Look at assembler code first:
>
> cc -S a.c
> cat a.s
> .file "a.c"
> .local nenv.1948
> .comm nenv.1948,8,4
> .section .rodata
> .LC0:
> .string "PATH=/bin"
> .text
>
> [skipped]
>
> As you may see, compiler puts "PATH=/bin" to the program's .rodata section
> which is placed to read only memory.
>
> If later you'll modify this single "PATH=/bin" (comes from "nenv" now) by
> *equals = '\0';
> ...
> *equals = '=';
> core dump happens, which simulated in my simple a.c example by
> nenv[0][4] = '\0';
>
> Just run it and got code dump.
The default setting for 'writable-string' changed in gcc; strings are
now NOT writable by default. While it may seem reasonable, setting
the programs environment to non-writable strings by manipulating the
environ pointer, then attempting to modify those strings, is bound to
fail. Since you've directly set the environment, there isn't much
that {set,put}env() could do, unless you want to get clever and have
them detect strings in .rodata and work around that.
The simplest solution may be a caution in the man page about stuffing
read-only strings into the environment.
--
Against stupidity the very gods Themselves contend in vain.
Friedrich Schiller
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?f83770800707160702g4cd08912pac63753681c7217e>