Re: Environment handling broken in /bin/sh with changes to {get, set, put}env()

On 7/13/07, Andrey Chernov <ache_at_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