Use of PR_SetEnv with static char[] causes assertion in free with tcsh

NEW
Assigned to

Status

NSPR
NSPR
7 years ago
7 years ago

People

(Reporter: Rich Megginson, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

7 years ago
If I have

static const char *envvar = "NSSNOFORK=DISABLED";
PR_SetEnv(envvar);

When this code is called in tcsh, tcsh aborts with the following assertion and stack trace:
> # su - foo
> free(0x173bd2) below bottom of memory. (memtop = 0x9898800 membot = 0x973e000)
> # gdb /bin/tcsh coredump
> (gdb) bt
> #0  0x00ed9416 in __kernel_vsyscall ()
> #1  0x006e02f1 in raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> #2  0x006e1d5e in abort () at abort.c:92
> #3  0x08084e6a in free (cp=0x8f2bd2) at tc.alloc.c:313
> #4  0x08063d83 in blkfree (av0=0x9e7d988) at sh.misc.c:158
> #5  0x0805af6f in tsetenv (name=0x80a3420 L"GROUP", val=0x9e32608
> L"valid_group") at sh.func.c:1725
> #6  0x0804d910 in main (argc=<value optimized out>, argv=0xbfa7e4f4) at
> sh.c:561

If I allocate char *envvar with PL_strdup() instead, everything works correctly.
(Assignee)

Comment 1

7 years ago
Thanks for the bug report.  On Unix, PR_SetEnv passes its
input argument directly to putenv().  See

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/src/misc/prenv.c&rev=3.12&mark=87,96#87

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/private/primpl.h&rev=3.93&mark=1907#1907

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/nsprpub/pr/include/md/_unixos.h&rev=3.41&mark=303#303

Your code snippet differs from the example in the putenv man page
only in the use of 'const':
http://pubs.opengroup.org/onlinepubs/009695399/functions/putenv.html

So you can try removing 'const'.  But I doubt that'll fix the
crash.

I found this CERT page, which recommends dynamically allocating
memory for the argument to putenv():
https://www.securecoding.cert.org/confluence/display/seccode/POS34-C.+Do+not+call+putenv()+with+a+pointer+to+an+automatic+variable+as+the+argument

But it also says:
  An automatic pointer to a static buffer would work as intended.

So this seems like a bug in tcsh?

Both the putenv() man page and the CERT page suggest using setenv()
instead.  Perhaps PR_SetEnv should use setenv() if it's available.
(Reporter)

Comment 2

7 years ago
Yes.  Perhaps use setenv() instead where available.
You need to log in before you can comment on or make changes to this bug.