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

Status: NEW

(NSPR :: NSPR, defect)

Description

[Rich Megginson]

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.

Comment 1

[Wan-Teh Chang]

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.

Comment 2

[Rich Megginson]

Yes.  Perhaps use setenv() instead where available.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.