Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 13 Jul 2007 20:27:42 +0400
From:      Andrey Chernov <ache@nagual.pp.ru>
To:        "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:  <20070713162742.GA16260@nagual.pp.ru>
In-Reply-To: <20070708171727.GA90490@nagual.pp.ru>
References:  <20070704215154.O77978@thor.farley.org> <20070705115816.GA50506@nagual.pp.ru> <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>

next in thread | previous in thread | raw e-mail | index | archive | help
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.

-- 
http://ache.pp.ru/



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20070713162742.GA16260>