Skip Menu |
Report information
Id: 127158
Status: resolved
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: krahmer [at] suse.com
mirabilos <tg [at] mirbsd.de>
stephane.chazelas [at] gmail.com
Cc: chet.ramey [at] case.edu
corydoras [at] ridiculousfish.com
herbert [at] gondor.apana.org.au
magicant [at] users.osdn.me
secalert [at] redhat.com
security [at] suse.de
team [at] security.debian.org
tg+security [at] mirbsd.org
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: (no value)
Type: (no value)
Perl Version: (no value)
Fixed In: (no value)



Date: Mon, 4 Jan 2016 23:21:10 +0000
Subject: security issue with multiple environment entries with the same name
To: perl5-security-report [...] perl.org, Thorsten Glaser <tg+security [...] mirbsd.org>, Chet Ramey <chet.ramey [...] case.edu>, Magicant <magicant [...] users.osdn.me>, corydoras [...] ridiculousfish.com, Herbert Xu <herbert [...] gondor.apana.org.au>, team [...] security.debian.org, secalert [...] redhat.com, security [...] suse.de
From: Stephane Chazelas <stephane.chazelas [...] gmail.com>
Download (untitled) / with headers
text/plain 5.6k
Hello, this is a follow-up on an issue I reported last year to the Debian security list and the sudo maintainer. As agreed with the Debian seclist, I'm widening the discussion to maintainers of software affected by that issue (or at least software best placed to mitigate it): perl, yash, bash, dash, mksh, fish. Debian, Suse and Redhat seclists are included also in their quality of security point of contacts for GNU libc (non-GNU systems are also affected though). It is a security issue in that it offers a way for attackers to bypass some sanitizing made to the environment. As noted by Florian Weimer, it's already partly known (https://www.securecoding.cert.org/confluence/display/c/ENV02-C.+Beware+of+multiple+environment+variables+with+the+same+effective+name), (issue tested on a current Debian testing system). The issue: The execve(file, argp[], envp[]) system call executes "file" and passes two lists of null-terminated strings. The second one is used to pass environment variables. By convention those strings are in the format "VARNAME=VARVALUE". There is typically no sanitizing done by the libc or the kernel between that list being passed to execve() and the executed command receiving it in its environ[] (except for specific env vars blocked by glibc under AT_SECURE). That envp[] may contain strings that don't follow that convention, or, and that's what's the issue here, nothing prevents one to pass a "VAR=value" *and* a "VAR=other-value". The essence of the problem is that when that happens, executed applications behave differently allowing env vars with the wrong value to get through. To test, you can compile this simple executable: #include <unistd.h> #include <stdio.h> #include <string.h> int main(int argc, char* argv[]) { int i; for (i = 2; strcmp(argv[i], "EOA"); i++); argv[i++] = 0; execve(argv[1], &argv[1], &argv[i]); perror(argv[1]); return 1; } Which is just a wrapper around execve(2). Called as: $ ./execve /bin/bash -c 'echo "$a"; env' EOA a=1 a=2 a=3 3 PWD=/tmp a=3 SHLVL=1 _=/usr/bin/env You can see here that bash sets its "a" variable from the *last* entry, and removes all the other ones. Other shells, languages or libraries dealing with the environment behave differently. The bash, dash, mksh, fish, rc, yash shells and perl use the *last* entry. ksh93, zsh and most other non-shells (libc's getenv, python, ruby, tcl...) use the *first* entry. Among shells, only zsh and yash preserve the other entries. yash and perl have a bug in that even though they use the *last* entry on input, upon assigning a variable, they update the first entry (also, perl uses the last entry for %ENV but the library it uses will typically use the first entry) $ ./execve /usr/bin/perl -le 'print $ENV{a}; $ENV{a} = "x";system("env")' EOA a=1 a=2 a=3 3 a=x a=2 a=3 $ ./execve /usr/bin/yash -c 'echo "$a"; a=x; /usr/bin/env' EOA a=1 a=2 a=3 3 a=x a=2 a=3 PWD=/tmp Other tools I tested are consistent on that aspect. When assigning a new value to an env var (for example with setenv()) that had multiple entries, with those tools that don't remove duplicates, only the first entry is updated. What that means for instance is that on those systems where sh is based on bash, dash or pdksh (that is most free software systems) a code (for instance for a setuid executable) that does: setenv("PATH=/bin:/sbin",1); /* sane value for PATH */ system("uname"); Would be vulnerable, because if called with "PATH=/tmp/evil", "PATH=/tmp/evil", that setenv() would only update the *first* PATH variable, while the "sh" invoked by system() would use the *last* one. In the case of perl, that allows bypassing the taint mode (set when perl is executed in setuids) in: $ ln -s /usr/bin/whoami uname $ ./execve /usr/bin/perl -Te 'system("uname;")' EOA PATH="$PWD" PATH="$PWD" Insecure $ENV{PATH} while running with -T switch at -e line 1. Good, perl didn't let us call uname with an unsafe PATH, but: $ ./execve /usr/bin/perl -Te '$ENV{PATH}="/bin";system("uname;")' EOA PATH="$PWD" PATH="$PWD" chazelas Here, our "evil" (a symlink to whoami) uname was called, because the second PATH variable was used. For sudo, that affected variables passed along by sudo (like LC_*) until Todd added some sanitizing: https://www.sudo.ws/repos/sudo/rev/d4dfb05db5d7 (preserves only the first instance). IMO, here is what should be done: - fix perl and yash so they get the first entry for a variable - bash, dash, mksh, fish changed so that they get the first entry as well to be consistent with putenv/setenv that update the first. - add some hardening to the libc so that under AT_SECURE, entries with duplicate env var names are removed (only first one retained). - since most shells already remove duplicates, yash and zsh could probably do it as well. - one could argue that setenv, putenv, or assigning env vars in other languages should also remove duplicates. Some more information as already given for the benefit of the new participants: - I've verified that while one can modify the source code of an openssh client to make it send several copies of an environment variable to the server, the server will only put one of them (the last one) in the environment of the remote shell. So the exploit cannot be carried out over ssh (at least openssh). - (only informational), the only case I know of an application passing two env vars with the same name was bash prior to shellshock where when you did: foo() { echo foo; }; foo=bar; export foo; export -f foo; cmd bash would pass both foo=bar and foo="() { echo foo; }" to cmd. (and if cmd was a bash script, both the foo variable and function would be imported). Since shellshock, bash now passes foo=bar and "BASH_FUNC_foo%%=() { echo foo; }" -- Stephane
CC: stephane.chazelas [...] gmail.com, Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Red Hat Product Security <secalert [...] redhat.com>
Date: Tue, 5 Jan 2016 15:57:57 +0000 (UTC)
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 1.5k
On Mon, 4 Jan 2016 23:21:21 GMT, stephane.chazelas@gmail.com wrote: […] Ouch! Good point. Show quoted text
>> - bash, dash, mksh, fish changed so that they get the first >> entry
Sorry, that’s not going to happen. Assignment must stay consistent within the shell language. If you run… foo() { echo $x; } x=1 x=2 x=3 foo … you expect it to show 3, not 1, too – independend of whether x is imported, exported or neither. I think the divergence is here that pdksh-based shells parse the entire environ into shell variables and then don’t use it any more so later lookup is a simple hash table lookup and fast, whereas libc getenv(3) stops at the first match to be fast. I would suggest letting the kernel deduplicate environ during exec… otherwise, making libc getenv(3) not stop at the first match is something libc implementors will be unhappy about but possibly have to do (does environ have a size limit?). Show quoted text
>> - one could argue that setenv, putenv, or assigning env vars in >> other languages should also remove duplicates.
This is something that, indeed, should be done, yes. Show quoted text
>> - (only informational), the only case I know of an application
[…] Show quoted text
>> bash would pass both foo=bar and foo="() { echo foo; }" to
Ah, okay. Anyway, thanks for uncovering this… “what fun” ☺ bye, //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
CC: stephane.chazelas [...] gmail.com, Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Red Hat Product Security <secalert [...] redhat.com>
Date: Tue, 5 Jan 2016 19:24:43 +0000 (UTC)
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 1.9k
Dixi quod… Show quoted text
>be unhappy about but possibly have to do (does environ >have a size limit?).
Yes, ARG_MAX protects us from this being a DoS vector. Show quoted text
>during exec… otherwise, making libc getenv(3) not stop >at the first match is something libc implementors will
Fun fact: making getenv/putenv read and set the last item breaks Perl, because Perl’s env import loop assigns the read environment variables to %ENV while the latter being magical already, so this… ~/execve /usr/bin/perl -le 'print $ENV{a}; $ENV{a} = "x"; print $ENV{a}; system("env")' EOA a=1 a=2 a=3 a=4 a=5 … always prints the second-to-last value. Similarily, from reading the code, it will react allergically to shrinking environ during that loop (due to duplicate removal). Show quoted text
>>> - one could argue that setenv, putenv, or assigning env vars in >>> other languages should also remove duplicates.
> >This is something that, indeed, should be done, yes.
This cannot be done, thus, without breaking existing binaries. Then I see only one way out of this misery: make the C startup (crt) remove the duplicates. Or the kernel, since a csu fix may not catch all cases (asm binaries) and would need to be replicated e.g. in ld.so as well. (Using MirBSD terms here.) In the meantime, I studied what POSIX has to say; it doesn’t touch on duplicate keys but suggests implementations to use a hashtable for getenv() for speed anyway (unless the application uses putenv()). (Note: I looked at Perl 5.8.8; later versions may or may not differ.) Although, one thing that *could* be done is to set the _first_ entry and garbage-collect any later ones in setenv/putenv, but if applications break like this I fear that may trigger bugs in other applications and believe normalisation in kernel/csu to be the best fix despite the startup cost for all applications. bye, //mirabilos -- (gnutls can also be used, but if you are compiling lynx for your own use, there is no reason to consider using that package) -- Thomas E. Dickey on the Lynx mailing list, about OpenSSL
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Tue, 5 Jan 2016 21:07:29 +0000 (UTC)
CC: stephane.chazelas [...] gmail.com, Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Red Hat Product Security <secalert [...] redhat.com>
Download (untitled) / with headers
text/plain 783b
Dixi quod… Show quoted text
>Although, one thing that *could* be done is to set >the _first_ entry and garbage-collect any later ones >in setenv/putenv
No, this won’t work either; Perl with -DPERL_USE_SAFE_PUTENV would use the first value then. So, short of normalising the environment before any user code is run, there’s no way to plug this problem without causing further breakage, unfortunately. (That being said, I found a couple of bugs in MirBSD libc’s env code, now I looked at it closely. Thanks.) bye, //mirabilos -- I believe no one can invent an algorithm. One just happens to hit upon it when God enlightens him. Or only God invents algorithms, we merely copy them. If you don't believe in God, just consider God as Nature if you won't deny existence. -- Coywolf Qi Hunt
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
To: Thorsten Glaser <tg [...] mirbsd.de>
CC: Red Hat Product Security <secalert [...] redhat.com>, Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Date: Tue, 5 Jan 2016 22:03:47 +0000
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 1.9k
2016-01-05 15:57:57 +0000, Thorsten Glaser: [...] Show quoted text
> >> - bash, dash, mksh, fish changed so that they get the first > >> entry
> > Sorry, that’s not going to happen. Assignment must stay > consistent within the shell language. > > If you run… > > foo() { echo $x; } > x=1 x=2 x=3 foo > > … you expect it to show 3, not 1, too – independend of > whether x is imported, exported or neither.
Hi Thorsten, That's something different. Of course in the shell command line: x=1 x=2 x=3 foo we've got 3 variable assignments for the same variable, so one overwrites the previous one. In the case of an external foo command, we end up to: execve("foo", ["foo"], ["x=3"]) not execve("foo", ["foo"], ["x=1", "x=2", "x=3"]) (note that the Bourne shell (and IIRC earlier versions of ash) parsed assignments from right to left, so you'd end up with "x=1" with those shells) What I'm talking of here is the processing of the environment the shell receives on startup. There, having more than one entry for an environment variable is a pathological case, variables are only meant to have *one* value. See how most shells (including pdksh/mksh do remove the duplicates). Shells that support exporting arrays (like rc, es, fish) do *not* do it by passing several a=1 a=2 (I suppose they can't rely on the order being preserved). zsh and ksh93 already do the right thing. The problem I'm talking of here (at least the particular attack vector I'm mentioning) is only because some shells (and perl) use the environment differently from the rest (from getenv/setenv to start with). If the problem is fixed (shells assign $foo from the *first* "foo=whatever" it receives and ignore (like zsh) or discard (like ksh93) further entries for that same variable (and everything else does the same), the problem goes away. Now that you mention hash tables, I wonder if putenv/setenv getting and updating the first entry and do not reorder the entries is the norm or not. -- Stephane
RT-Send-CC: team [...] security.debian.org, corydoras [...] ridiculousfish.com, secalert [...] redhat.com, chet.ramey [...] case.edu, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, tg+security [...] mirbsd.org, rt-deliver-to-perl5-security-report [...] rt.perl.org, team [...] security.debian.org, secalert [...] redhat.com, magicant [...] users.osdn.me, herbert [...] gondor.apana.org.au, tg+security [...] mirbsd.org, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com
Download (untitled) / with headers
text/plain 430b
On Mon Jan 04 15:22:04 2016, stephane.chazelas@gmail.com wrote: Show quoted text
> - fix perl and yash so they get the first entry for a variable
The first of the attached patches does that by reversing the order we initialize %ENV. Show quoted text
> - one could argue that setenv, putenv, or assigning env vars in > other languages should also remove duplicates.
The second patch attempts to remove duplicates of an environment variable when it's set. Tony
Subject: 0001-perl-127158-reverse-initialization-of-ENV.patch
From 9db49ead97699afd2030ae0c121a9ae7eb4a2a2f Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 6 Jan 2016 10:30:08 +1100 Subject: [perl #127158] reverse initialization of %ENV On most platforms, if there are duplicate environment variables, getenv() and setenv() get/modify the first entry found, make %ENV consistent with that. --- perl.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/perl.c b/perl.c index b8d98ff..d33def9 100644 --- a/perl.c +++ b/perl.c @@ -4319,8 +4319,11 @@ S_init_postdump_symbols(pTHX_ int argc, char **argv, char **env) if (env) { char *s, *old_var; SV *sv; - for (; *env; env++) { - old_var = *env; + char **env_start = env; + while (*env) + ++env; + while (env > env_start) { + old_var = *--env; if (!(s = strchr(old_var,'=')) || s == old_var) continue; @@ -4330,10 +4333,10 @@ S_init_postdump_symbols(pTHX_ int argc, char **argv, char **env) (void)strupr(old_var); *s = '='; #endif - sv = newSVpv(s+1, 0); - (void)hv_store(hv, old_var, s - old_var, sv, 0); - if (env_is_not_environ) - mg_set(sv); + sv = newSVpv(s+1, 0); + (void)hv_store(hv, old_var, s - old_var, sv, 0); + if (env_is_not_environ) + mg_set(sv); } } #endif /* USE_ENVIRON_ARRAY */ -- 2.1.4
Subject: 0002-perl-127158-remove-duplicates-in-my_setenv.patch
From 57bed4c34abe49405a03403cf3d4d26699ec0ad0 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 6 Jan 2016 11:43:18 +1100 Subject: [perl #127158] remove duplicates in my_setenv() It's possible for environ to contain duplicate definitions of an environment variable, to ensure a child process sees only the definition we remove any duplicates when a given variable is set. The implementations of unsetenv() I've tested removed all of the definitions, but POSIX is silent on how duplicate names are handled, so explicitly unsetenv() the name until we're sure it's gone. For the PERL_USE_SAFE_PUTENV case without unsetenv there's not really anything we can do. This needs tests, assuming I can find a way to to them. --- util.c | 74 ++++++++++++++++++++++++++++++++++++++++++------------------------ 1 file changed, 47 insertions(+), 27 deletions(-) diff --git a/util.c b/util.c index 17b62dd..8f4514b 100644 --- a/util.c +++ b/util.c @@ -2148,30 +2148,47 @@ Perl_my_setenv(pTHX_ const char *nam, const char *val) tmpenv[max] = NULL; environ = tmpenv; /* tell exec where it is now */ } - if (!val) { - safesysfree(environ[i]); - while (environ[i]) { - environ[i] = environ[i+1]; - i++; + if (val) { + if (!environ[i]) { /* does not exist yet */ + environ = (char**)safesysrealloc(environ, (i+2) * sizeof(char*)); + environ[i+1] = NULL; /* make sure it's null terminated */ + } + else + safesysfree(environ[i]); + nlen = strlen(nam); + vlen = strlen(val); + + environ[i] = (char*)safesysmalloc((nlen+vlen+2) * sizeof(char)); + /* all that work just for this */ + my_setenv_format(environ[i], nam, nlen, val, vlen); + + /* look for a duplicate entry, if any */ + for (i++; environ[i]; i++) { + if (strnEQ(environ[i],nam,len) && environ[i][len] == '=') + break; } -#ifdef __amigaos4__ - goto my_setenv_out; -#else - return; -#endif - } - if (!environ[i]) { /* does not exist yet */ - environ = (char**)safesysrealloc(environ, (i+2) * sizeof(char*)); - environ[i+1] = NULL; /* make sure it's null terminated */ } - else + + if (environ[i]) { /* environ[i] is a match */ + I32 from = i+1; + + assert(strnEQ(environ[i],nam,len) && environ[i][len] == '='); + safesysfree(environ[i]); - nlen = strlen(nam); - vlen = strlen(val); - environ[i] = (char*)safesysmalloc((nlen+vlen+2) * sizeof(char)); - /* all that work just for this */ - my_setenv_format(environ[i], nam, nlen, val, vlen); + /* remove any duplicate definitions */ + while (environ[from]) { + if (strnEQ(environ[from],nam,len) && environ[from][len] == '=') { + safesysfree(environ[from]); + ++from; + } + else { + environ[i++] = environ[from++]; + } + } + /* copy the NULL */ + environ[i] = environ[from]; + } } else { # endif /* This next branch should only be called #if defined(HAS_SETENV), but @@ -2180,9 +2197,10 @@ Perl_my_setenv(pTHX_ const char *nam, const char *val) */ # if defined(__CYGWIN__)|| defined(__SYMBIAN32__) || defined(__riscos__) || (defined(__sun) && defined(HAS_UNSETENV)) # if defined(HAS_UNSETENV) - if (val == NULL) { - (void)unsetenv(nam); - } else { + /* ensure any duplicates are removed */ + while (PerlEnv_getenv(nam)) + unsetenv(nam); + if (val) { (void)setenv(nam, val, 1); } # else /* ! HAS_UNSETENV */ @@ -2190,10 +2208,12 @@ Perl_my_setenv(pTHX_ const char *nam, const char *val) # endif /* HAS_UNSETENV */ # else # if defined(HAS_UNSETENV) - if (val == NULL) { - if (environ) /* old glibc can crash with null environ */ - (void)unsetenv(nam); - } else { + /* ensure any duplicates are removed */ + if (environ) { /* old glibc can crash with null environ */ + while (PerlEnv_getenv(nam)) + unsetenv(nam); + } + if (val) { const int nlen = strlen(nam); const int vlen = strlen(val); char * const new_env = -- 2.1.4
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
From: Sebastian Krahmer <krahmer [...] suse.com>
CC: Thorsten Glaser <tg [...] mirbsd.de>, corydoras [...] ridiculousfish.com, team [...] security.debian.org, herbert [...] gondor.apana.org.au, Chet Ramey <chet.ramey [...] case.edu>, security [...] suse.de, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, perl5-security-report [...] perl.org
Subject: Re: [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Wed, 6 Jan 2016 11:00:42 +0100
Download (untitled) / with headers
text/plain 1.3k
On Tue, Jan 05, 2016 at 10:03:47PM +0000, Stephane CHAZELAS wrote: [...] Show quoted text
> > The problem I'm talking of here (at least the particular attack > vector I'm mentioning) is only because some shells (and perl) > use the environment differently from the rest (from > getenv/setenv to start with). > > If the problem is fixed (shells assign $foo from the *first* > "foo=whatever" it receives and ignore (like zsh) or discard > (like ksh93) further entries for that same variable (and > everything else does the same), the problem goes away. > > Now that you mention hash tables, I wonder if putenv/setenv > getting and updating the first entry and do not reorder the > entries is the norm or not.
Worse. putenv/setenv reallocate and move stuff across the heap, resetting __environ and may leave envp[] argument of main() out of sync. (think threads) I think the only thing we can do is sticking to what POSIX says about getenv/putenv etc. and making perl etc. aware that dups may happen and remove them on init (not ignoring them! that already caused root exploits via openbsd rtld double LD_PRELOAD). glibc properly handles insecure dups on AT_SECURE. Maybe POSIX allows to erase dups for any startup by ld.so, which would be prefered. -s -- ~ perl self.pl ~ $_='print"\$_=\47$_\47;eval"';eval ~ krahmer@suse.com - SuSE Security Team
Subject: Re: [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Wed, 6 Jan 2016 13:08:19 +0000
CC: Thorsten Glaser <tg [...] mirbsd.de>, corydoras [...] ridiculousfish.com, team [...] security.debian.org, herbert [...] gondor.apana.org.au, Chet Ramey <chet.ramey [...] case.edu>, security [...] suse.de, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, perl5-security-report [...] perl.org
To: Sebastian Krahmer <krahmer [...] suse.com>
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Download (untitled) / with headers
text/plain 2.5k
2016-01-06 11:00:42 +0100, Sebastian Krahmer: [...] Show quoted text
> Worse. putenv/setenv reallocate and move stuff across the heap, > resetting __environ and may leave envp[] argument of main() > out of sync. (think threads)
[...] But can any of the env entries that a program *received* (as opposed to the ones added later with setenv/putenv) be reordered with glibc (or other libcs)? I've not managed to. Can setenv("x", "value") update anything else than the *first* entry? Or that upon the next execve(), that "x=value" may not be the first one? Show quoted text
> I think the only thing we can do is sticking to what > POSIX says about getenv/putenv etc. and making perl etc. > aware that dups may happen and remove them on init (not ignoring them! that > already caused root exploits via openbsd rtld double LD_PRELOAD).
[...] Well, the problem is that in: setenv("PATH=sane-value"); execlp("perl-script"); or system("cmd"); setenv updates only one of the PATH entries (in my experience the first), but perl or sh use a different one. Even if perl is modified to use only one and ignore the rest (and most sh do that), it has to be the same one as set by setenv(). Here, if we were to cast blame, that would partly on setenv() that fails to update *all* the PATH entries (or set one PATH env var and remove the other ones), or on bash/mksh/dash/fish that get a different PATH from the one set by setenv() (or the kernel's execve() for not removing duplicates). Show quoted text
> glibc properly handles insecure dups on AT_SECURE.
[...] What do you mean? They don't seem to be removed here (Linux Mint 17.3) for a setuid executable: $ ll env -rwsr-x--- 1 root chazelas 27232 Nov 27 2014 env* $ ./execve ./env EOA PATH=/tmp/evil PATH=/tmp/evil PATH=/tmp/evil PATH=/tmp/evil $ ./execve ./env PATH=/usr/bin env EOA PATH=/tmp/evil PATH=/tmp/evil PATH=/usr/bin PATH=/tmp/evil $ /lib/x86_64-linux-gnu/libc-2.19.so GNU C Library (Ubuntu EGLIBC 2.19-0ubuntu6.6) stable release version 2.19, by Roland McGrath et al. Copyright (C) 2014 Free Software Foundation, Inc. This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. Compiled by GNU CC version 4.8.2. Compiled on a Linux 3.13.11 system on 2015-02-25. Available extensions: crypt add-on version 2.1 by Michael Glad and others GNU Libidn by Simon Josefsson Native POSIX Threads Library by Ulrich Drepper et al BIND-8.2.3-T5B libc ABIs: UNIQUE IFUNC For bug reporting instructions, please see: <https://bugs.launchpad.net/ubuntu/+source/eglibc/+bugs>. (according to ltrace, that "env" uses putenv(3)). -- Stephane
From: Sebastian Krahmer <krahmer [...] suse.com>
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
CC: corydoras [...] ridiculousfish.com, team [...] security.debian.org, security [...] suse.de, Chet Ramey <chet.ramey [...] case.edu>, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, Thorsten Glaser <tg [...] mirbsd.de>, perl5-security-report [...] perl.org, herbert [...] gondor.apana.org.au
Subject: Re: [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Wed, 6 Jan 2016 14:33:13 +0100
Download (untitled) / with headers
text/plain 2.7k
On Wed, Jan 06, 2016 at 01:08:19PM +0000, Stephane CHAZELAS wrote: Show quoted text
> 2016-01-06 11:00:42 +0100, Sebastian Krahmer: > [...]
> > Worse. putenv/setenv reallocate and move stuff across the heap, > > resetting __environ and may leave envp[] argument of main() > > out of sync. (think threads)
> [...] > > But can any of the env entries that a program *received* (as > opposed to the ones added later with setenv/putenv) be reordered > with glibc (or other libcs)? I've not managed to. > > Can setenv("x", "value") update anything else than the *first* > entry? Or that upon the next execve(), that "x=value" may not be > the first one?
Thats implementation dependend of corse, so we have to assume the bad case that it may be re-ordered. If not now, maybe in future implementations. Thats why I voted for removing (erasing) dups rather than ignoring them (and also because of the de-sync envp[] thing). Show quoted text
>
> > I think the only thing we can do is sticking to what > > POSIX says about getenv/putenv etc. and making perl etc. > > aware that dups may happen and remove them on init (not ignoring them! that > > already caused root exploits via openbsd rtld double LD_PRELOAD).
> [...] > > Well, the problem is that in: > > setenv("PATH=sane-value"); > execlp("perl-script"); > or system("cmd"); > > setenv updates only one of the PATH entries (in my experience > the first), but perl or sh use a different one. Even if perl is > modified to use only one and ignore the rest (and most sh do > that), it has to be the same one as set by setenv().
Yes, see above about _removing_ dups. Also note that this example is insecure either way since best practices is to either clearenv() and only then using setenv() or to pass the array itself to execve(). For exactly the reason of junk/dups that may be inherited. Using a polluted environment and then calling some setenv's never was secure. White lists vs. black lists approach. Show quoted text
> > Here, if we were to cast blame, that would partly on setenv() > that fails to update *all* the PATH entries (or set one PATH env > var and remove the other ones), or on bash/mksh/dash/fish that > get a different PATH from the one set by setenv() (or the > kernel's execve() for not removing duplicates). >
> > glibc properly handles insecure dups on AT_SECURE.
> [...] > > What do you mean? > > They don't seem to be removed here (Linux Mint 17.3) for a > setuid executable:
Removing any and all occurences of insecure env vars, including any dups. TL;DR: I ack the problem of environment variable dups, but patching perl etc. to ignore them is no solution to me. Either patch perl etc. to remove the dups on early startup so they cant do harm, or "enhance" ld.so to do so (if POSIX and ABI permits that). Sebastian -- ~ perl self.pl ~ $_='print"\$_=\47$_\47;eval"';eval ~ krahmer@suse.com - SuSE Security Team
Date: Wed, 6 Jan 2016 13:40:11 +0000 (UTC)
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>, corydoras [...] ridiculousfish.com, team [...] security.debian.org, herbert [...] gondor.apana.org.au, Chet Ramey <chet.ramey [...] case.edu>, security [...] suse.de, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, perl5-security-report [...] perl.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Sebastian Krahmer <krahmer [...] suse.com>
Download (untitled) / with headers
text/plain 1.1k
Sebastian Krahmer dixit: Show quoted text
>Worse. putenv/setenv reallocate and move stuff across the heap, >resetting __environ and may leave envp[] argument of main() >out of sync. (think threads)
POSIX says that: – environment is not thread-safe at all anyway – envp out-of-sync is to be expected – applications shall use getenv/setenv/unsetenv, but the historic usage of looking at environ[] is still allowed Show quoted text
>glibc properly handles insecure dups on AT_SECURE. Maybe POSIX allows >to erase dups for any startup by ld.so, which would be prefered.
POSIX doesn’t say a single word about duplicates (I looked). I also think fixing this in the startup (kernel, ld.so, crt0) is the only option not breaking any existing applicatiions. As for the first-vs.-last debate: I asked a random selection of Linux and Mac OSX users with some programming knowledge, and they unambigously said they expect the *last* occurrence of the key to be used, n̲o̲t̲ the first. Tony Cook via RT dixit: Show quoted text
>The first of the attached patches does that by reversing the order >we initialize %ENV.
Please do *not* do that! bye, //mirabilos -- This space for rent.
CC: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>, corydoras [...] ridiculousfish.com, team [...] security.debian.org, security [...] suse.de, Chet Ramey <chet.ramey [...] case.edu>, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, herbert [...] gondor.apana.org.au
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Sebastian Krahmer <krahmer [...] suse.com>
Date: Wed, 6 Jan 2016 13:52:54 +0000 (UTC)
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 1.6k
Sebastian Krahmer dixit: Show quoted text
>On Wed, Jan 06, 2016 at 01:08:19PM +0000, Stephane CHAZELAS wrote:
Show quoted text
>> But can any of the env entries that a program *received* (as >> opposed to the ones added later with setenv/putenv) be reordered >> with glibc (or other libcs)? I've not managed to.
That’s possible, and I had that as next step after… Show quoted text
>> Can setenv("x", "value") update anything else than the *first* >> entry? Or that upon the next execve(), that "x=value" may not be >> the first one?
… this, which was very easy to do in MirBSD libc, but it turns out that doing this breaks at least Perl. Show quoted text
>Thats why I voted for removing (erasing) dups rather than ignoring them
Exactly. Show quoted text
>(and also because of the de-sync envp[] thing).
POSIX says envp[] is not available because it would be de-synched; setenv(3) is allowed to change environ. Show quoted text
>Yes, see above about _removing_ dups. Also note that this example >is insecure either way since best practices is to either clearenv() and only
clearenv() is explicitly not in POSIX, but you can use execve() or change environ in the application (which is also permissible). Show quoted text
>then using setenv() or to pass the array itself to execve(). For exactly >the reason of junk/dups that may be inherited.
Indeed, which is why the de-duplication must happen at application start before any user code is run, possibly in the kernel, otherwise in the startup code. Show quoted text
>ld.so to do so (if POSIX and ABI permits that).
POSIX doesn’t say anything about duplicates, and I’d say go, but don’t forget static executables (oh, and those compiled with other runtimes… FreePascal comes to mind, or anything Assembly). bye, //mirabilos -- This space for rent.
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
To: Thorsten Glaser <tg [...] mirbsd.de>
CC: Sebastian Krahmer <krahmer [...] suse.com>, corydoras [...] ridiculousfish.com, team [...] security.debian.org, herbert [...] gondor.apana.org.au, Chet Ramey <chet.ramey [...] case.edu>, security [...] suse.de, Red Hat Product Security <secalert [...] redhat.com>, magicant [...] users.osdn.me, perl5-security-report [...] perl.org
Date: Wed, 6 Jan 2016 15:42:52 +0000
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
2016-01-06 13:40:11 +0000, Thorsten Glaser: [...] Show quoted text
> As for the first-vs.-last debate: I asked a random selection > of Linux and Mac OSX users with some programming knowledge, > and they unambigously said they expect the *last* occurrence > of the key to be used, n̲o̲t̲ the first.
[...] Not sure what you asked them exactly, but I'd expect if you ask them which one should getenv() or setenv() update, they would say the first and that's what most things do (getenv/setenv, python, lua, ruby, tcl, zsh, AT&T ksh at least). That's a mute point as that's something that is not meant to happen anyway. And which one it is doesn't matter anyway as long as everyone agrees on it. But as it looks like we may not be able to have a guarantee of order, I agree it's probably safer to enforce unique entries at a lower level (kernel, crt...) on startup. Would that break things though? Until now, at least on GNU/Linux, argv and envp are interchangeable. It's still possible some things may use the environment in unconventional fashion? (unlikely when many shells do remove the unconventional entries). -- Stephane
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Stéphane Chazelas <stephane.chazelas [...] gmail.com>
CC: Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, perl5-security-report [...] perl.org, tg [...] mirbsd.org
Date: Wed, 6 Jan 2016 18:30:07 +0000 (UTC)
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 3.7k
stephane.chazelas@gmail.com via RT dixit: Show quoted text
>2016-01-06 13:40:11 +0000, Thorsten Glaser:
Show quoted text
>> As for the first-vs.-last debate: I asked a random selection >> of Linux and Mac OSX users with some programming knowledge, >> and they unambigously said they expect the *last* occurrence >> of the key to be used, n̲o̲t̲ the first.
Show quoted text
>Not sure what you asked them exactly, but I'd expect if you ask
I asked them that, when I pass x=1 x=2 x=3 to a program in the environment, what they’d expect the value of x to be. Show quoted text
>them which one should getenv() or setenv() update, they would >say the first and that's what most things do (getenv/setenv, >python, lua, ruby, tcl, zsh, AT&T ksh at least).
This is probably either due to actually using getenv/setenv or not having thought about it enough. Consider: env = {} env['x'] = '1' env['x'] = '2' env['x'] = '3' I fail to see why there ought to be a distinction between environment imports and in-language assignments in the first place; additionally, for things like JSON, the last key wins as well, so this is overall more consistent. Show quoted text
>And which one it is doesn't matter anyway as long as everyone >agrees on it.
Getting everyone to agree probably will be a hell of fun… (including my own stubbornness) ☺ Show quoted text
>guarantee of order, I agree it's probably safer to enforce >unique entries at a lower level (kernel, crt...) on startup.
Right. Show quoted text
>Would that break things though? Until now, at least on >GNU/Linux, argv and envp are interchangeable. It's still
You mean environ and envp. They are interchangable until the first setenv() with a new value was called, but no longer, from that point. But look at how main() gets called. I’m using the MirBSD code as reference here, as I’m intimately familiar with it, but I assume most ELF-based systems do the same. Let’s look at a statically linked executable. The kernel loads the ELF binary and jumps to its entry point, which is usually some variant of the _start symbol, as gcc calls ld with -e _start (or -e __start). That is called “raw”, i.e. not with a function frame. The implementation of __start then reads some arguments from registers (such as ps_strings) and stack (argc, for example) and calculates envp as the stack address just behind argc plus argc+1 times the size of a pointer. Then it constructs a C stack frame and calls some less machine-specific function which does a couple of things, depending on the OS. For MirBSD, that is: • set environ to envp • set __progname as basename(argv[0]) • store ps_strings aside • if profiling: atexit(_mcleanup) and call monstartup • call atexit(_fini); and _init(); (_init() does things like constructors) and its last line is: • exit(main(argc, argv, environ)); That means that the envp main() sees is not the envp passed by the OS (actually, the OS doesn't pass one, it just places it on the stack right after the argv array) but whatever environ is set to. Furthermore, we only d̲e̲duplicate here, i.e. we don’t increase the size of the environment. environ[0], environ[1], etc. are all places on the stack, pointing to strings also on the stack. To deduplicate, we merely have to memmove() things inside the environ array around, which means that we don’t even need to change the value of environ/envp but just its contents. I remember looking at how Linux does things, and it also just places the environment pointers on the stack after the argument vector pointers IIRC. Of course, doing this in the kernel would fix this the best way as no code duplication (ld.so for dynamically linked executables, crt0 for statically linked ones, and some unknown amount of custom runtimes, different C libraries (dietlibc, klibc, µClibc-ng, musl, …) and who knows what else) is needed. bye, //mirabilos -- This space for rent.
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Wed, 6 Jan 2016 20:18:46 +0000
From: Stéphane Chazelas <stephane.chazelas [...] gmail.com>
To: Thorsten Glaser <tg [...] mirbsd.de>
CC: Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, perl5-security-report [...] perl.org, tg [...] mirbsd.org
Download (untitled) / with headers
text/plain 2.2k
2016-01-06 18:30:07 +0000, Thorsten Glaser: [...] Show quoted text
> >Would that break things though? Until now, at least on > >GNU/Linux, argv and envp are interchangeable. It's still
> > You mean environ and envp.
[...] FWIW, I did mean argv[] and envp[], as in they are just two lists of arbitrary strings. Some applications, may use envp[] for other purposes than passing a list of environment variables. [...] Show quoted text
> That means that the envp main() sees is not the envp > passed by the OS (actually, the OS doesn't pass one, > it just places it on the stack right after the argv > array) but whatever environ is set to.
But environ would be the address of the first pointer to the first env string, that's something put there by execve(). Show quoted text
> Furthermore, we only d̲e̲duplicate here, i.e. we don’t > increase the size of the environment. environ[0], > environ[1], etc. are all places on the stack, pointing > to strings also on the stack. To deduplicate, we merely > have to memmove() things inside the environ array around, > which means that we don’t even need to change the value > of environ/envp but just its contents.
Just the content of the pointers, the duplicate env values would still stay on the stack, but no longer reached. Show quoted text
> I remember looking at how Linux does things, and it also > just places the environment pointers on the stack after > the argument vector pointers IIRC.
Yes, see http://unix.stackexchange.com/questions/91561/ps-full-command-is-too-long/91562#91562 the bottom of the stack is: argc argv-pointers envp-pointers argv-strings envp-strings. Show quoted text
> Of course, doing this in the kernel would fix this the > best way as no code duplication (ld.so for dynamically > linked executables, crt0 for statically linked ones, > and some unknown amount of custom runtimes, different > C libraries (dietlibc, klibc, µClibc-ng, musl, …) and > who knows what else) is needed.
[...] Yes. True. It can potentially break things though. On the other hand, changing bash,dash,yash,mksh,fish,perl so they get the first duplicate would be harmless. That may not completely fix the problem, but nobody has offered a counter-example yet where for example: putenv("PATH=sane-value"); system("foo"); with a fixed "sh" (one behaving like zsh or ksh93) can be fooled on current systems. -- Stephane
Date: Wed, 6 Jan 2016 20:44:34 +0000 (UTC)
Subject: Re: [perl #127158] [security@suse.de] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: Stéphane Chazelas <stephane.chazelas [...] gmail.com>, perl5-security-report [...] perl.org, Chet Ramey <chet.ramey [...] case.edu>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, mirabilos <tg [...] mirbsd.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
Download (untitled) / with headers
text/plain 1.4k
(Why isn’t dgk (the Korn in ksh) in the Cc list, anyway?) stephane.chazelas@gmail.com via RT dixit: Show quoted text
>lists of arbitrary strings. Some applications, may use envp[] >for other purposes than passing a list of environment variables.
Well POSIX doesn’t guarantee it anyway. Show quoted text
>But environ would be the address of the first pointer to the >first env string, that's something put there by execve().
And that is not going to change when we deduplicate. Show quoted text
>Just the content of the pointers, the duplicate env values would >still stay on the stack, but no longer reached.
Exactly. Even better if the kernel deduplicates, of course. That needs the various kernels’ hackers to do it… Show quoted text
>the bottom of the stack is: > >argc argv-pointers envp-pointers argv-strings envp-strings.
Right, and we just make the envp-pointers a bit shorter (and fill it up to the original size with NULL pointers). Show quoted text
>It can potentially break things though.
Every change can potentially break things. This is the place where I believe the least amount of them breaks ;) Show quoted text
>On the other hand, changing bash,dash,yash,mksh,fish,perl so >they get the first duplicate would be harmless. That may not
I don’t currently see enough reasons to change mksh to give the first value precedence. Later assignments a̲l̲w̲a̲y̲s̲ override earlier assignments. Furthermore, this would make 'sh' and '(env;cat)|sh' out of sync which can cause even more “fun”… bye, //mirabilos -- This space for rent.
Date: Wed, 06 Jan 2016 20:10:02 +0000
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: hv [...] crypt.org
To: perl5-security-report [...] perl.org
CC: hv [...] crypt.org
"Tony Cook via RT" <perl5-security-report@perl.org> wrote: :On Mon Jan 04 15:22:04 2016, stephane.chazelas@gmail.com wrote: :> - fix perl and yash so they get the first entry for a variable : :The first of the attached patches does that by reversing the order :we initialize %ENV. Technically, this looks good to me, independent of the discussion on whether it's desirable. :> - one could argue that setenv, putenv, or assigning env vars in :> other languages should also remove duplicates. : :The second patch attempts to remove duplicates of an environment variable :when it's set. Other than minor comments below, this also looks good. I'm a bit worried that Perl_vmssetenv() explicitly documents that it will set/remove only the first copy found, it'd be useful to get some insight into the nature of this under VMS and what effect this change for other platforms will have on portability. [...] :+ nlen = strlen(nam); For later: note that this (and the same in alternate branches below) is the same as len above, other than type; surely we can avoid calling strlen() twice for these? :+ if (environ[i]) { /* environ[i] is a match */ I think this could do with a clearer comment, maybe something like: /* environ[i] now points to an entry to be removed; remove it, and any * further instances of the same name. */ : # if defined(__CYGWIN__)|| defined(__SYMBIAN32__) || defined(__riscos__) || (defined(__sun) && defined(HAS_UNSETENV)) Note this context line will conflict on rebase, since blead now has PERL_DARWIN too. :+ /* ensure any duplicates are removed */ :+ while (PerlEnv_getenv(nam)) :+ unsetenv(nam); I'm nervous of infinite loops here and in the unsetenv+putenv case below. I'd really like to see a test that would exercise these in the obscure platform smokes, even if we have to release before we can see those. : # else /* ! HAS_UNSETENV */ I think this and the putenv() case below need to comment that they won't (and can't) kill dupes. Longer term, there might be value in a build option that would refuse to build in these cases. Hugo
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 911b
On Wed Jan 06 06:13:54 2016, mirabilos wrote: Show quoted text
> Tony Cook via RT dixit: >
> >The first of the attached patches does that by reversing the order > >we initialize %ENV.
> > Please do *not* do that!
One problem with the current behaviour is that the internals of perl (and XS code) that use getenv() and perl code can see two different values for an environment variable, eg: $ execve ./perl -Ilib -MPOSIX -le 'print $ENV{LANG}; print setlocale("");' EOA LANG=en_US.UTF-8 LANG=en_AU.UTF-8 en_AU.UTF-8 en_US.UTF-8 Reversing the order fixes that when getenv() works in what seems to me to be the most obvious way (a linear search from the start of environ.) Another change I'm considering is aborting under -T if a duplicate is found. I can see that causing problems if anyone is creating an envp[] for execve() by copying environ[] and appending new definitions to the end without removing duplicates. Tony
CC: krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
To: Tony Cook via RT <perl5-security-report [...] perl.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
Date: Thu, 7 Jan 2016 04:13:30 +0000 (UTC)
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Tony Cook via RT dixit: Show quoted text
>On Wed Jan 06 06:13:54 2016, mirabilos wrote:
>> Tony Cook via RT dixit: >>
>> >The first of the attached patches does that by reversing the order >> >we initialize %ENV.
>> >> Please do *not* do that!
> >One problem with the current behaviour is that the internals of perl >(and XS code) that use getenv() and perl code can see two different >values for an environment variable, eg:
Unless that has changed since Perl 5.8.8, if you enable putenv, this will additionally break things (try with three or more values… in my test I didn’t reverse but assigning to %ENV during import would overwrite values when Perl order and setenv order don’t match). Show quoted text
>I can see that causing problems if anyone is creating an envp[] for >execve() by copying environ[] and appending new definitions to the end >without removing duplicates.
Another point in favour of last-one-wins, and another point in favour of deduplicating between handover to the kernel on execve() and running any user code in the new process. bye, //mirabilos -- This space for rent.
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Thu, 7 Jan 2016 12:04:22 +0000
To: Tony Cook via RT <perl5-security-report [...] perl.org>
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
CC: krahmer [...] suse.com, tg [...] mirbsd.de, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Download (untitled) / with headers
text/plain 1.6k
2016-01-06 20:02:44 -0800, Tony Cook via RT: [...] Show quoted text
> One problem with the current behaviour is that the internals > of perl (and XS code) that use getenv() and perl code can see > two different values for an environment variable, eg: > > $ execve ./perl -Ilib -MPOSIX -le 'print $ENV{LANG}; print setlocale("");' EOA LANG=en_US.UTF-8 LANG=en_AU.UTF-8 > en_AU.UTF-8 > en_US.UTF-8
Yes, I had already mentioned that in my initial report but forgot to add it when /widening/ the discussion, sorry about that. Original report attached in case there's more I forgot to mention (my memory is not as good as it used to be...). Show quoted text
> Reversing the order fixes that when getenv() works in what > seems to me to be the most obvious way (a linear search from > the start of environ.)
Yes, and one of the big questions is indeed "can we rely (in practice, in existing implementations, I know POSIX gives no guarantee there) on getenv() returning the first entry and on setenv()/putenv() not re-shuffling the entries (at least the order of entries received on the last execve() preserved for the next execve())?" Show quoted text
> Another change I'm considering is aborting under -T if a duplicate is found.
Sounds reasonable. As I noted earlier though, pre-shellshock bashs could genuinely have two instances of a variable (but then again, those duplicates would be stripped by some tools like most shells, so it would already be broken) Show quoted text
> I can see that causing problems if anyone is creating an > envp[] for execve() by copying environ[] and appending new > definitions to the end without removing duplicates.
[...] If they do, they're broken already, as that duplicate entry won't be accessible by most things including getenv(). -- Stephane
From: Stephane Chazelas <stephane.chazelas [...] gmail.com>
Date: Fri, 20 Nov 2015 15:21:46 +0000
To: "Todd C. Miller" <Todd.Miller [...] courtesan.com>, team [...] security.debian.org
Subject: sudo/setuids and multiple environment entries with the same name
Download (untitled) / with headers
text/plain 3.7k
Hello, this is not an issue with sudo, but I think something sudo (and the libc/dynamic linker at least in setuid/setgid...) should guard against. Consider this little C program: #define _GNU_SOURCE #include <unistd.h> int main(int argc, char* argv[]) { char *e[] = {"LC_ALL=tr_TR.UTF-8", "LC_ALL=tr_TR.UTF-8", 0}; execvpe(argv[1], &argv[1], e); return 1; } to be used as: ./a.out cmd... (on non-GNU systems, replace execvpe with execvp and cmd with /path/to/cmd). That calls cmd with LC_ALL=tr_TR.UTF-8 twice in the environment. If I run it as: ./a.out sudo cmd That variable is still passed twice. Now, if we have a variable passed twice like that, the behaviour varies between applications. getenv("LC_ALL") will return the first one, $ENV{"LC_ALL"} in perl will return the last one, but $ENV{"LC_ALL"} = "C" will update the first one (!). Some shells $LC_ALL (zsh, AT&T ksh) will get the first one, dash, bash, yash, pdksh the last one. Of those, all but zsh and yash will remove the duplicates. perl/ruby/python won't. setenv() and putenv() on GNU, like zsh, yash, perl, ruby, python when assigning to LC_ALL will update the first one and leave the other ones untouched. What that means is that if I have a program that does: putenv("LC_ALL=C"); /* make sure we're in a sane locale */ /* or setenv("LC_ALL", "C", 1) */ system("something"); Or: #!/usr/bin/perl $ENV{LC_ALL}="C"; system("something"); Or: #! /usr/bin/ruby ENV["LC_ALL"]="C"; exec("something"); Or: #!/bin/zsh LC_ALL_C something If the "sh" called by system() is based on dash/bash/pdksh which is usually the case on FOSS OSes, or "something" is a sh/perl script or calls a sh/perl script, then LC_ALL will still be tr_TR.UTF-8. I like the tr_TR.UTF-8 locale in examples, because at least on Debian, that's one locale where uppercase i is not I (so echo evil | grep -i EVIL fails), the decimal separator is ",", thousand separator ".", month/day/AM/PM have multi-byte characters... and of course all the security issues related to UTF-8 and non-C collation. LC_ALL is one of the variables passed along by sudo at least on Debian, but we could also have problems with any variable (like PATH) by any other setuid/setgid program. Basically, the vulnerability we have here is a way to bypass environment sanitising in privilege escalation contexts by passing the env var several times. I've not made a review of what software may be impacted, but there's bound to be some. PAM modules, setuid/setgids especially should be checked against that. IMO, the libc and sudo should strip those duplicates at least for setuid/setgids. bash/dash/mksh already strip duplicates but should retain the first one to be consistent with getenv(). perl should also be fixed to get the first one in its $ENV (that's definitly a bug there as it updates the first one when you assign to $ENV elements, and when it calls setlocale(), it's the first one that is used). One could also argue that putenv/setenv or assigning env vars in any of those languages should remove duplicates. PoC: $ cat a.c #define _GNU_SOURCE #include <unistd.h> int main(int argc, char* argv[]) { char *e[] = {"LC_ALL=tr_TR.UTF-8", "LC_ALL=tr_TR.UTF-8", 0}; execvpe(argv[1], &argv[1], e); return 1; } $ cat sanitize.c #include <stdlib.h> #include <unistd.h> int main(int argc, char*argv[]) {putenv("LC_ALL=C"); execvp(argv[1], &argv[1]);} $ make a cc a.c -o a $ make sanitize cc sanitize.c -o sanitize $ ./a sudo ./sanitize sh -c 'echo evil | grep -i EVIL' $ ./a sudo ./sanitize sh -c 'date' Cum Kas 20 14:21:49 GMT 2015 $ ./a sudo perl -e '$ENV{LC_ALL}="C"; system("date")' Fri Nov 20 14:22:25 GMT 2015 $ ./a sudo perl -e '$ENV{LC_ALL}="C"; system("date>&2")' Cum Kas 20 14:22:31 GMT 2015 $ ./a sudo zsh -c 'LC_ALL=C sh -c date' Cum Kas 20 14:23:32 GMT 2015 -- Stephane
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 693b
It seems to me like the sanest, simplest, and least confusing thing is to use Tony Cook's two patches to (a) make perl's env have the same contents you'd get from a linear search and (b) clean up dupes on set. Tony and I briefly discussed the possibility of cleaning up the environment itself on boot, but it didn't seem like a good way forward. As for barfing on ambiguous environments: * I'd love doing that always if I felt more sure that it was nearly never the case in sane operation (but I really don't feel that way) * I'm not keen on making it depend on -T, as it turns -T into "more security" rather than "taint" * ...but that doesn't mean I think it's a bad idea, either -- rjbs
To: perl5-security-report [...] perl.org
From: Jarkko Hietaniemi <jhi [...] iki.fi>
Date: Mon, 11 Jan 2016 18:42:36 -0500
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 585b
On Monday-201601-11 18:37, Ricardo SIGNES via RT wrote: Show quoted text
> * I'd love doing that always if I felt more sure that it was nearly never the case in sane operation (but I really don't feel that way) > * I'm not keen on making it depend on -T, as it turns -T into "more security" rather than "taint" > * ...but that doesn't mean I think it's a bad idea, either
Croak by default but have an environment variable env PERL_ALLOW_DUP_ENV_FULL_SPEED_AHEAD_AND_DAMN_THE_TORPEDOES=1 ... would be playing on the "safe by default" side, while still allowing a foolhardy captain to steam ahead?
To: Jarkko Hietaniemi <jhi [...] iki.fi>
From: Tony Cook <tony [...] develop-help.com>
CC: perl5-security-report [...] perl.org
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Tue, 12 Jan 2016 10:49:47 +1100
Download (untitled) / with headers
text/plain 778b
On Mon, Jan 11, 2016 at 06:42:36PM -0500, Jarkko Hietaniemi wrote: Show quoted text
> On Monday-201601-11 18:37, Ricardo SIGNES via RT wrote:
> >* I'd love doing that always if I felt more sure that it was nearly never the case in sane operation (but I really don't feel that way) > >* I'm not keen on making it depend on -T, as it turns -T into "more security" rather than "taint" > >* ...but that doesn't mean I think it's a bad idea, either
> > Croak by default but have an environment variable > > env PERL_ALLOW_DUP_ENV_FULL_SPEED_AHEAD_AND_DAMN_THE_TORPEDOES=1 ... > > would be playing on the "safe by default" side, while still allowing a > foolhardy captain to steam ahead?
Do you know if VMS can have multiple definitions of the same name in the environment in normal cases? Tony
Date: Mon, 11 Jan 2016 18:57:16 -0500
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Jarkko Hietaniemi <jhi [...] iki.fi>
To: Tony Cook <tony [...] develop-help.com>
CC: "perl5-security-report [...] perl.org" <perl5-security-report [...] perl.org>
Download (untitled) / with headers
text/plain 1.2k
I don't know VMS that intimately but I would suspect it does not, since I think env there is basically a system-level lookup table, instead of a manually-diddled single block of memory. In fact, it is a hierarchy of lookup tables. If we don't have Craig in perl-security, we should. On Mon, Jan 11, 2016 at 6:49 PM, Tony Cook <tony@develop-help.com> wrote: Show quoted text
> On Mon, Jan 11, 2016 at 06:42:36PM -0500, Jarkko Hietaniemi wrote:
>> On Monday-201601-11 18:37, Ricardo SIGNES via RT wrote:
>> >* I'd love doing that always if I felt more sure that it was nearly never the case in sane operation (but I really don't feel that way) >> >* I'm not keen on making it depend on -T, as it turns -T into "more security" rather than "taint" >> >* ...but that doesn't mean I think it's a bad idea, either
>> >> Croak by default but have an environment variable >> >> env PERL_ALLOW_DUP_ENV_FULL_SPEED_AHEAD_AND_DAMN_THE_TORPEDOES=1 ... >> >> would be playing on the "safe by default" side, while still allowing a >> foolhardy captain to steam ahead?
> > Do you know if VMS can have multiple definitions of the same name in > the environment in normal cases? > > Tony
-- There is this special biologist word we use for 'stable'. It is 'dead'. -- Jack Cohen
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.7k
On Mon Jan 11 15:43:05 2016, jhi wrote: Show quoted text
> On Monday-201601-11 18:37, Ricardo SIGNES via RT wrote:
> > * I'd love doing that always if I felt more sure that it was nearly > > never the case in sane operation (but I really don't feel that way) > > * I'm not keen on making it depend on -T, as it turns -T into "more > > security" rather than "taint" > > * ...but that doesn't mean I think it's a bad idea, either
> > Croak by default but have an environment variable > > env PERL_ALLOW_DUP_ENV_FULL_SPEED_AHEAD_AND_DAMN_THE_TORPEDOES=1 ... > > would be playing on the "safe by default" side, while still allowing a > foolhardy captain to steam ahead?
So, my concern with that is something like this: If ambiguous environments are created by (possibly half-baked programs written in) shells, and those are the only places where this is going to approach "common" in the wild, then we're going to see zero reports of failure until things are failing on production systems. Lots of people smoke their applications and libraries on bleadperl, but nobody "smokes" their production systems, cron jobs, glue, and so on. They also tend to run that stuff against system perl, meaning that they won't see the breakage when v5.x.0 comes out, but some years later when their scripts are ported to the new company standard, Excited Egret. Changes that will affect "the stuff sysadmins do" are the changes where I feel we must be the most careful. So, with that in mind, are we better off having perl bail or having it change to use the first value? It's not entirely clear. Either way (putting aside the case where (X=1) is found multiple times in env), the behavior changes. My gut says that we're better off having programs still run, although I'm not sure I have any strong argument to make in favor of it. -- rjbs
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 247b
On Mon Jan 11 15:57:32 2016, jhi wrote: Show quoted text
> If we don't have Craig in perl-security, we should.
Tony noted his absence and I sent him an invite earlier this evening. If he declines to join I'll at least get him to consult on this ticket. -- rjbs
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.9k
On Mon Jan 11 16:00:13 2016, rjbs wrote: Show quoted text
> On Mon Jan 11 15:57:32 2016, jhi wrote:
> > If we don't have Craig in perl-security, we should.
> > Tony noted his absence and I sent him an invite earlier this evening. > If he declines to join I'll at least get him to consult on this > ticket.
I'm here (at least in the RT queue). The Perl %ENV hash on VMS is constructed from a list of one or more of logical names, CLI symbols, and the CRTL environ array in a specified order. The first two are hashes so guaranteed unique within each when accessed from within the same mode (more on that in a sec). Don't know offhand about environ; it's probably like most Unix systems that haven't been specifically updated for this vulnerability. As I think Hugo was hinting at up-thread, there may be a special variant of the bug on VMS deriving from the fact that %ENV is concocted from multiple sources and even if each is unique within itself, the flattened view in %ENV could have multiple definitions of the same key behind it. If I have a logical name FOO and an environ element FOO, deleting $ENV{FOO} will remove whichever one is first in my search list and leave the other. The next lookup will find what's left. I guess this could be changed to delete it from all locations, not just the first one found, but that would be a change in documented behavior. I need to think more about this and come up with a test case or two. Also, logical names have layers called modes. It's more or less like the Windows registry where less privileged versions of the same key sit "in front of" the more privileged versions. Come to think of it Windows environment variables also exist at the user and system levels. In all of these cases, a key can have a less privileged translation and a more privileged one, and I think deleting the first will leave the second. Not sure if that's exploitable in any way, but for the sake of thoroughness it is a form of having two translations for the same key.
To: Ricardo SIGNES via RT <perl5-security-report [...] perl.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
CC: krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Tue, 12 Jan 2016 22:26:43 +0000 (UTC)
Download (untitled) / with headers
text/plain 542b
Ricardo SIGNES via RT dixit: Show quoted text
>So, with that in mind, are we better off having perl bail or having it >change to use the first value? It's not entirely clear. Either way
Considering the entire scenario (not just Perl) it’s probably best to not change any existing applications (e.g. Perl or the shells) but change the OSes to sanitise the environment, and possibly, if at all, warn (or, in some strict mode, die) if duplicates are found (but consider those warnings could also impact programs). bye, //mirabilos -- This space for rent.
To: perl5-security-report [...] perl.org
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Jarkko Hietaniemi <jhi [...] iki.fi>
Date: Wed, 13 Jan 2016 09:25:50 -0500
On Monday-201601-11 18:59, Ricardo SIGNES via RT wrote: Show quoted text
> On Mon Jan 11 15:43:05 2016, jhi wrote:
>> On Monday-201601-11 18:37, Ricardo SIGNES via RT wrote:
>>> * I'd love doing that always if I felt more sure that it was nearly >>> never the case in sane operation (but I really don't feel that way) >>> * I'm not keen on making it depend on -T, as it turns -T into "more >>> security" rather than "taint" >>> * ...but that doesn't mean I think it's a bad idea, either
>> >> Croak by default but have an environment variable >> >> env PERL_ALLOW_DUP_ENV_FULL_SPEED_AHEAD_AND_DAMN_THE_TORPEDOES=1 ... >> >> would be playing on the "safe by default" side, while still allowing a >> foolhardy captain to steam ahead?
> > So, my concern with that is something like this: > > If ambiguous environments are created by (possibly half-baked programs written in) shells, and those are the only places where this is going to approach "common" in the wild, then we're going to see zero reports of failure until things are failing on production systems. > > Lots of people smoke their applications and libraries on bleadperl, but nobody "smokes" their production systems, cron jobs, glue, and so on. They also tend to run that stuff against system perl, meaning that they won't see the breakage when v5.x.0 comes out, but some years later when their scripts are ported to the new company standard, Excited Egret. Changes that will affect "the stuff sysadmins do" are the changes where I feel we must be the most careful. > > So, with that in mind, are we better off having perl bail or having it change to use the first value? It's not entirely clear. Either way (putting aside the case where (X=1) is found multiple times in env), the behavior changes. My gut says that we're better off having programs still run, although I'm not sure I have any strong argument to make in favor of it.
Personally I find it somewhat concerning to prioritize not breaking existing code over fixing security issues. But since I cannot make up my mind over how serious this particular issue is, I don't feel particularly strongly either way.
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.5k
On Wed Jan 06 12:59:12 2016, hv wrote: Show quoted text
> :+ /* ensure any duplicates are removed */ > :+ while (PerlEnv_getenv(nam)) > :+ unsetenv(nam); > > I'm nervous of infinite loops here and in the unsetenv+putenv case > below.
Me too. I haven't built with this patch yet but I think that could easily cause an infinite loop on VMS because %ENV is a tied hash to the real environment outside Perl and you wouldn't normally have privileges to delete everything in it. Here's a taste for how the layers work where I've defined the same key in multiple different logical name tables: $ define/job PERLTMP JOB $ define/process PERLTMP PROCESS $ define/user PERLTMP USER $ perl -e "while (exists $ENV{PERLTMP}) {print qq/$ENV{PERLTMP}\n/; delete $ENV{PERLTMP};}" USER PROCESS JOB If I had also defined PERLTMP system-wide and run the above code without privileges it would loop infinitely because it would not be able to delete the last entry. Having multiple definitions for the same key in different layers is a natural and normal way to do things on VMS and lots of things depend on that working. Now, in addition to all of the above I could also have a definition for PERLTMP in the CRTL environ array that would be invisible in %ENV if the logical name was defined but visible if the logical name was not defined. That's documented behavior. If the problem is just that a hidden value could avoid taint checks, I don't think that happens for any of these cases. I infer that there's more to it than that but don't yet have a very good handle on what it is.
Date: Wed, 13 Jan 2016 21:47:56 -0500
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
To: Jarkko Hietaniemi <jhi [...] iki.fi>
CC: perl5-security-report [...] perl.org
Download (untitled) / with headers
text/plain 868b
* Jarkko Hietaniemi <jhi@iki.fi> [2016-01-13T09:25:50] Show quoted text
> Personally I find it somewhat concerning to prioritize not breaking existing > code over fixing security issues. But since I cannot make up my mind over > how serious this particular issue is, I don't feel particularly strongly > either way.
I agree with you on both counts, here: we need to take action to fix things if they're really broken security-wise, but I'm not sure how they are. There's a related question: if we have a security problem because existing code can behave unexpectedly, what exactly would "expectedly" mean? The "die on ambiguous environment" solution is a concession to the notion that nobody knows. I think nobody knows, which makes it appealing. The "don't break things" angle makes me want to downgrade to "we will tell you what to expect: act like a linear scan." -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

From: Jarkko Hietaniemi <jhi [...] iki.fi>
Date: Thu, 14 Jan 2016 06:49:35 -0500
CC: perl5-security-report [...] perl.org
To: Ricardo Signes <perl.security [...] rjbs.manxome.org>
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 357b
On Wednesday-201601-13 21:47, Ricardo Signes wrote: Show quoted text
> I agree with you on both counts, here: we need to take action to fix things if > they're really broken security-wise, but I'm not sure how they are.
Just for the sake of argument, privilege escalation (local or remote) would be a security issue that I think should overrule the "not breaking" rule.
Date: Thu, 14 Jan 2016 12:13:42 +0000 (UTC)
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Jarkko Hietaniemi via RT <perl5-security-report [...] perl.org>
CC: krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Download (untitled) / with headers
text/plain 772b
Jarkko Hietaniemi via RT dixit: Show quoted text
>On Wednesday-201601-13 21:47, Ricardo Signes wrote:
>> I agree with you on both counts, here: we need to take action to fix things if >> they're really broken security-wise, but I'm not sure how they are.
> >Just for the sake of argument, privilege escalation (local or remote) >would be a security issue that I think should overrule the "not >breaking" rule.
Although, this is an OS-level issue that cannot easily be fixed just in various userspace tools. Though, if you wanted to fix Perl, ensure that the deduplication happens before putenv is called (or the environ pointers are manipulated, in the !SAFE_PUTENV case). Then, choose either the first or last to win (I’d say the last). bye, //mirabilos -- This space for rent.
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.1k
On Tue Jan 05 17:12:24 2016, tonyc wrote: Show quoted text
> On Mon Jan 04 15:22:04 2016, stephane.chazelas@gmail.com wrote:
> > - fix perl and yash so they get the first entry for a variable
> > The first of the attached patches does that by reversing the order > we initialize %ENV.
diff --git a/perl.c b/perl.c index b8d98ff..d33def9 100644 --- a/perl.c +++ b/perl.c @@ -4319,8 +4319,11 @@ S_init_postdump_symbols(pTHX_ int argc, char **argv, char **env) if (env) { I've been going a bit crazy trying to figure out how this code will ever fire. As far as I can see init_postdump_symbols is only ever called from perl_parse or S_parse_body via perl_parse. The one and only call to perl_parse in perlmain.c looks like: exitstatus = perl_parse(my_perl, xs_init, argc, argv, (char **)NULL); So that last argument passing in the environment is always NULL, and has been since 5.002 Beta 1: http://perl5.git.perl.org/perl.git/commit/4633a7c4bad06b471d9310620b7fe8ddd158cccd?f=miniperlmain.c So any references to the environment in perl_parse or anything it calls (including S_init_postdump_symbols) appear to be vestigial. What am I missing?
Date: Fri, 15 Jan 2016 11:35:40 +1100
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: ;, rt-deliver-to-perl5-security-report [...] rt.perl.org
From: Tony Cook <tony [...] develop-help.com>
To: "Craig A. Berry via RT" <bugs-comment [...] rt.perl.org>
Download (untitled) / with headers
text/plain 1.2k
On Thu, Jan 14, 2016 at 04:06:54PM -0800, Craig A. Berry via RT wrote: Show quoted text
> On Tue Jan 05 17:12:24 2016, tonyc wrote:
> > On Mon Jan 04 15:22:04 2016, stephane.chazelas@gmail.com wrote:
> > > - fix perl and yash so they get the first entry for a variable
> > > > The first of the attached patches does that by reversing the order > > we initialize %ENV.
> > diff --git a/perl.c b/perl.c > index b8d98ff..d33def9 100644 > --- a/perl.c > +++ b/perl.c > @@ -4319,8 +4319,11 @@ S_init_postdump_symbols(pTHX_ int argc, char **argv, char **env) > if (env) { > > I've been going a bit crazy trying to figure out how this code will ever fire. As far as I can see init_postdump_symbols is only ever called from perl_parse or S_parse_body via perl_parse. The one and only call to perl_parse in perlmain.c looks like: > > exitstatus = perl_parse(my_perl, xs_init, argc, argv, (char **)NULL); > > So that last argument passing in the environment is always NULL, and has been since 5.002 Beta 1: > > http://perl5.git.perl.org/perl.git/commit/4633a7c4bad06b471d9310620b7fe8ddd158cccd?f=miniperlmain.c > > So any references to the environment in perl_parse or anything it calls (including S_init_postdump_symbols) appear to be vestigial. What am I missing?
Around line 4308: if (!env) env = environ; Tony
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 532b
On Thu Jan 14 16:36:10 2016, tonyc wrote: Show quoted text
> On Thu, Jan 14, 2016 at 04:06:54PM -0800, Craig A. Berry via RT wrote:
Show quoted text
> > So any references to the environment in perl_parse or anything it > > calls (including S_init_postdump_symbols) appear to be vestigial. > > What am I missing?
> > Around line 4308: > > if (!env) > env = environ;
Aha, thanks. On VMS we define NO_ENVIRON_ARRAY in vmsish.h, which means USE_ENVIRON_ARRAY will be false, which explains why I stepped right past all this in the debugger without seeing it.
To: Thorsten Glaser <tg [...] mirbsd.de>, Jarkko Hietaniemi via RT <perl5-security-report [...] perl.org>
CC: chet.ramey [...] case.edu, krahmer [...] suse.com, stephane.chazelas [...] gmail.com, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Chet Ramey <chet.ramey [...] case.edu>
Date: Fri, 15 Jan 2016 10:15:14 -0500
Download (untitled) / with headers
text/plain 987b
On 1/14/16 7:13 AM, Thorsten Glaser wrote: Show quoted text
> Jarkko Hietaniemi via RT dixit: >
>> On Wednesday-201601-13 21:47, Ricardo Signes wrote:
>>> I agree with you on both counts, here: we need to take action to fix things if >>> they're really broken security-wise, but I'm not sure how they are.
>> >> Just for the sake of argument, privilege escalation (local or remote) >> would be a security issue that I think should overrule the "not >> breaking" rule.
> > Although, this is an OS-level issue that cannot easily be > fixed just in various userspace tools.
I agree that this is something that should be fixed in the C library or the kernel. Doesn't it seem like the fact that we're proposing to fix so many separate pieces and tools implies that those proposals fix the problem in the wrong place? -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, ITS, CWRU chet@case.edu http://cnswww.cns.cwru.edu/~chet/
From: Florian Weimer <fw [...] deneb.enyo.de>
To: Chet Ramey <chet.ramey [...] case.edu>
CC: Thorsten Glaser <tg [...] mirbsd.de>, Jarkko Hietaniemi via RT <perl5-security-report [...] perl.org>, krahmer [...] suse.com, stephane.chazelas [...] gmail.com, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Date: Fri, 15 Jan 2016 20:01:19 +0100
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 574b
* Chet Ramey: Show quoted text
> I agree that this is something that should be fixed in the C library or the > kernel.
I agree. I would welcome a discussion on libc-alpha. Any fix will have to be subject to public review because it's a pretty significant change. One problem is that we have to implement very early in process startup in glibc, where nothing of the library proper is available for use. The issue has been in the public domain, as a security issue, since the mid-90s at least. It's mentioned in the second edition of “Practical UNIX & Internet Security”. Florian
Date: Fri, 15 Jan 2016 19:22:56 +0000 (UTC)
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Florian Weimer via RT <perl5-security-report [...] perl.org>
Download (untitled) / with headers
text/plain 526b
Florian Weimer via RT dixit: Show quoted text
>change. One problem is that we have to implement very early in >process startup in glibc, where nothing of the library proper is >available for use.
Which is why I suggested to do it in the kernel instead. bye, //mirabilos -- Stéphane, I actually don’t block Googlemail, they’re just too utterly stupid to successfully deliver to me (or anyone else using Greylisting and not whitelisting their ranges). Same for a few other providers such as Hotmail. Some spammers (Yahoo) I do block.
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.2k
On Fri Jan 15 07:32:44 2016, chet.ramey@case.edu wrote: Show quoted text
> On 1/14/16 7:13 AM, Thorsten Glaser wrote:
Show quoted text
> > Although, this is an OS-level issue that cannot easily be > > fixed just in various userspace tools.
> > I agree that this is something that should be fixed in the C library > or the kernel.
Towards that end, and FWIW, here's a quick and dirty program that doesn't involve any utilities but just execs itself to demonstrate whether the execve implementation preserves or eliminates duplicate keys. Here I'm running on OS X 10.11.2 and the presence of both A=1 and A=3 in the output indicates that no deduplication is done. $ cat execve_env_dups.c #include <stdio.h> #include <stdlib.h> #include <unistd.h> int main(int argc, char *argv[], char *envp[]) { char *newargv[] = { NULL, "printenv", NULL }; char *newenvp[] = { "A=1", "B=2", "A=3", "C=4", NULL }; if (argc == 1) { newargv[0] = argv[0]; execve(argv[0], newargv, newenvp); perror("execve"); /* execve() shouldn't return */ exit(EXIT_FAILURE); } else { int i = 0; while (envp[i] != NULL) { printf("%s\n", envp[i]); i++; } exit(EXIT_SUCCESS); } } $ ./execve_env_dups A=1 B=2 A=3 C=4
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Fri, 15 Jan 2016 21:25:40 +0100
From: Florian Weimer <fw [...] deneb.enyo.de>
To: Thorsten Glaser <tg [...] mirbsd.de>
CC: Florian Weimer via RT <perl5-security-report [...] perl.org>, krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Download (untitled) / with headers
text/plain 613b
* Thorsten Glaser: Show quoted text
> Florian Weimer via RT dixit: >
>>change. One problem is that we have to implement very early in >>process startup in glibc, where nothing of the library proper is >>available for use.
> > Which is why I suggested to do it in the kernel instead.
From a kernel perspective, the environment list is just another argument list. It's elements do not even have to have the KEY=VALUE structure. Breaking that seems to be a bit drastic. It would need some sort of policy knob, I think. Is the general feeling to apply this sanitization only on an AT_SECURE/SUID boundary, or always? Florian
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Fri, 15 Jan 2016 20:38:23 +0000 (UTC)
CC: krahmer [...] suse.com, stephane.chazelas [...] gmail.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
From: Thorsten Glaser <tg [...] mirbsd.de>
To: Florian Weimer via RT <perl5-security-report [...] perl.org>
Download (untitled) / with headers
text/plain 989b
Florian Weimer via RT dixit: Show quoted text
>>From a kernel perspective, the environment list is just another
>argument list. It's elements do not even have to have the KEY=VALUE
POSIX requires that structure, though; on the other hands, keys can be pretty much arbitrary, so the kernel should deduplicate everything up to the first = encountered and leave the string as is when it has no = at all. Show quoted text
>Is the general feeling to apply this sanitization only on an >AT_SECURE/SUID boundary, or always?
Always, this has too great a chance of introducing variations in behaviour that can be unexpected, and we never know right now what other tools could use the environment for restrictions in some time, or even now. bye, //mirabilos -- Stéphane, I actually don’t block Googlemail, they’re just too utterly stupid to successfully deliver to me (or anyone else using Greylisting and not whitelisting their ranges). Same for a few other providers such as Hotmail. Some spammers (Yahoo) I do block.
To: Chet Ramey <chet.ramey [...] case.edu>
CC: Jarkko Hietaniemi via RT <perl5-security-report [...] perl.org>, krahmer [...] suse.com, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Date: Fri, 15 Jan 2016 22:00:15 +0000
Download (untitled) / with headers
text/plain 2.6k
2016-01-15 10:15:14 -0500, Chet Ramey: [...] Show quoted text
> > Although, this is an OS-level issue that cannot easily be > > fixed just in various userspace tools.
> > I agree that this is something that should be fixed in the C library or the > kernel. > > Doesn't it seem like the fact that we're proposing to fix so many separate > pieces and tools implies that those proposals fix the problem in the wrong > place?
[...] De-duplicating at the kernel level would be a change of kernel API. At the kernel level, envp passed to execve() is just a list of arbitrary null-delimited strings. And it's documented that that list of strings be received by the executed command asis. The fact that those strings are treated as "var=value" strings is just a user-land convention of POSIX-like systems using that kernel API. bash/mksh/perl/yash/dash use that kernel API and implement that userland convention but have what can be regarded as a bug (especially true for yash and perl that don't deduplicate) in that when there are duplicates, they pick a different ones from the rest of userland. So assuming that kernel API is maintained (and I can't imagine all the kernels where those applications are ported would agree to change), I think those applications should be fixed. That change (pick the first entry from a linear look-up assuming that's what the rest of userland is doing) is harmless there (those applications implement that userland convention, where there shouldn't be duplicates so picking any of those duplicates shouldn't matter except that everyone should agree on which) and I beleive relatively simple. Yes, the kernel could enforce that userland convention, but then we'd need to define the scope: - remove duplicate variable names? - remove entries without "="? - remove entries with empty variable name? - remove entries that don't match POSIX variable names ([[:alpha:]_][[:alnum:]_]*), in which charset? - What about state-full character encodings where one string can have several different byte representations? And we'd need all kernels (Linux, FreeBSD, OpenBSD, NetBSD, AIX, HP/UX, Solaris, skipping the 214 other ones perl/bash have been ported to) to agree to the change or bash/pdksh/dash/perl/yash would still be broken on the other ones. What about non-POSIX userlands that use that envp differently? Doing it at the crt/libc level unconditionaly I'd say may be borderline acceptable as well. That's why I suggested the deduplicating be done at the libc level as a security hardening measure upon AT_SECURE (in detected privilege elevation contexts like setuid/setgid...) *in addition* to fixing bash/dash/pdksh/yash/perl. -- Stephane
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
To: Florian Weimer via RT <perl5-security-report [...] perl.org>
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org
Date: Fri, 15 Jan 2016 22:04:05 +0000
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Download (untitled) / with headers
text/plain 471b
2016-01-15 11:20:22 -0800, Florian Weimer via RT: Show quoted text
> * Chet Ramey: >
> > I agree that this is something that should be fixed in the C library or the > > kernel.
> > I agree. I would welcome a discussion on libc-alpha.
[...] Florian and other distro seclists, Please feel free to get libc(s) or kernel(s) or other distro seclists in the loop. I'm not a security specialist, you guys will have the best contacts and better notions on how to handle that. -- Stephane
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Fri, 15 Jan 2016 22:14:25 +0000 (UTC)
From: Thorsten Glaser <tg [...] mirbsd.de>
To: "stephane.chazelas [...] gmail.com via RT" <perl5-security-report [...] perl.org>
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Download (untitled) / with headers
text/plain 1.7k
stephane.chazelas@gmail.com via RT dixit: Show quoted text
>De-duplicating at the kernel level would be a change of kernel >API. At the kernel level, envp passed to execve() is just a list >of arbitrary null-delimited strings. > >And it's documented that that list of strings be received by the >executed command asis.
For the one kernel you likely looked at, and even then, maybe. Show quoted text
>bash/mksh/perl/yash/dash use that kernel API and implement that >userland convention but have what can be regarded as a bug
I’m going to ignore that paragraph. Show quoted text
> - remove duplicate variable names?
Yes. Show quoted text
> - remove entries without "="?
Probably not. Show quoted text
> - remove entries with empty variable name?
Good catch, will need thought. Show quoted text
> - remove entries that don't match POSIX variable names > ([[:alpha:]_][[:alnum:]_]*), in which charset?
No, because POSIX environment is arbitrary C string. Show quoted text
>And we'd need all kernels (Linux, FreeBSD, OpenBSD, NetBSD, AIX, >HP/UX, Solaris, skipping the 214 other ones perl/bash have been >ported to) to agree to the change or bash/pdksh/dash/perl/yash >would still be broken on the other ones.
Yes, they would, but that’s true nevertheless. Show quoted text
>What about non-POSIX userlands that use that envp differently?
POSIX requires environ, not envp (and actually getenv/setenv). Show quoted text
>Doing it at the crt/libc level unconditionaly I'd say may be >borderline acceptable as well.
And this is where you get hypocrite – crt level is the same as kernel level, from the application PoV. bye, //mirabilos -- Stéphane, I actually don’t block Googlemail, they’re just too utterly stupid to successfully deliver to me (or anyone else using Greylisting and not whitelisting their ranges). Same for a few other providers such as Hotmail. Some spammers (Yahoo) I do block.
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Date: Fri, 15 Jan 2016 22:25:27 +0000 (UTC)
To: "stephane.chazelas [...] gmail.com via RT" <perl5-security-report [...] perl.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Download (untitled) / with headers
text/plain 1.7k
Dixi quod… Show quoted text
>stephane.chazelas@gmail.com via RT dixit:
>>Doing it at the crt/libc level unconditionaly I'd say may be >>borderline acceptable as well.
> >And this is where you get hypocrite – crt level is the same as >kernel level, from the application PoV.
More importantly: To any userspace application after this change has been implemented, whether it was done in csu/ld.so/libc or in the kernel will look *exactly* the same (content of memory environ/envp points to). To any userspace application from *before* this change, only doing it in the kernel will transparently fix the issue for them. I’m in both situations – portable tool developer (mksh) and whole operating system developer (MirBSD, and, to a lesser degree, involved with Debian) – and can say that this must in any case be fixed on the OS level. What the tools do, if any (mksh’s environment normali‐ sation stems from internal representation, not an extra security step, and, now more than ever, will not change), is somewhat orthogonal to this and not required for this discussion; I expect most portable tools that do care about older or exotic (VMS?) operating environments to implement some sort of deduplication but we cannot rely on that for contemporary, developed, OSes. Now back to implementation details… wait, did anyone change POSIX in the meantime? It now reads: […] If more than one string in an environment of a process has the same name, the consequences are undefined. setenv() fails if name is empty, so any entries beginning with a = can also be removed. bye, //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 1.7k
I've reflected a bit more on this and concluded that as far as handling duplicates in the C run-time's environ array, nothing special needs to be done for VMS. We don't currently run perl.c:S_init_postdump_symbols, but I think we should and will enable it shortly. At which point Tony's first patch to take the first entry passed in when there are duplicates will do the same for us as anybody else. VMS doesn't and mustn't use util.c:Perl_my_setenv because of all the special magic in vms.c:Perl_vmssetenv, so Tony's second patch won't have any effect on VMS. As of cda27dcf504146747, vms.c:Perl_vmssetenv now uses the CRTL's unsetenv when removing something from the environ array. unsetenv has been available for twenty years and is documented to remove all versions when there's more than one. I've tested it and it works as advertised, so we're ok there. Now to the fact that %ENV is concocted from multiple sources on VMS and one of those sources (logical names) has multiple layers, and that deleting a key in %ENV could thus leave another value for the same key visible. There are multiple problems with trying to change that to delete all versions: 1.) It breaks documented behavior. 2.) It contradicts native expectations about how the environment works. 3.) It can never work reliably because successive versions are increasingly likely to be defined in places a user program does not have privileges to modify. 4.) It seems to me a contentious change (even if I have only myself to contend with) and I don't think I'm going to have it all figured (or even understand what exploits if any, are possible) before the contentious change freeze two days from now. So I think I need to leave things as is for now, but at least there is nothing VMS-related that stands in the way of proceeding with other changes discussed here.
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
CC: chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
To: Red Hat Product Security <secalert [...] redhat.com>
Date: Thu, 21 Jan 2016 17:39:08 +0000
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Download (untitled) / with headers
text/plain 1.2k
2016-01-21 12:08:38 -0500, Red Hat Product Security: [...] Show quoted text
> However, this appears to be one of the issues where it's very important to have > a general consensus on how exactly the fix should work and look like. I do > believe that finding this consensus will be a huge part of the work that needs > to be done to resolve this issue. > > I don't think that what we have now, a collection of CCed parties, is a > suitable place for discussing this. Thus, I think we should ultimately move > this discussion to a more suitable place, such as a public mailing list. > libc-alpha was suggested before.
[...] perl is possibly the most severely impacted (bypass of taint mode) and runs on a variety of Unix-likes and non-Unix-likes. From the discussion here, they've discussed changing their %ENV so it picks the first entry like getenv(3) on "get" and remove duplicates on "set". Once that's done that would probably fix it on most systems, then we can discuss the libc hardening. Personaly, I'm happy for the discussion to go to libc-alpha as long as the maintainers of perl and bash (ported to and used on most systems, and primary "vectors") are happy about it. If anybody has any objection to the issue being made public, they should raise it now. -- Stephane
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
Date: Thu, 21 Jan 2016 18:14:02 +0100
CC: tg+security [...] mirbsd.org, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, tg [...] mirbsd.de, security [...] suse.de, perl5-security-report [...] perl.org, chet.ramey [...] case.edu, team [...] security.debian.org, Todd.Miller [...] courtesan.com, tg [...] mirbsd.org, corydoras [...] ridiculousfish.com, krahmer [...] suse.com
To: Red Hat Product Security <secalert [...] redhat.com>
From: Florian Weimer <fw [...] deneb.enyo.de>
Download (untitled) / with headers
text/plain 1.2k
* Red Hat Product Security: Show quoted text
> In order to keep things moving: Red Hat has assigned CVE-2016-0748 > to a fix planned to be in glibc. I expect the libc fix to be some > kind of a "milestone", which would allow us to go ahead and poke at > all the other stuff that may need separate fixing. > > However, this appears to be one of the issues where it's very > important to have a general consensus on how exactly the fix should > work and look like. I do believe that finding this consensus will be > a huge part of the work that needs to be done to resolve this issue. > > I don't think that what we have now, a collection of CCed parties, > is a suitable place for discussing this. Thus, I think we should > ultimately move this discussion to a more suitable place, such as a > public mailing list. libc-alpha was suggested before. > > Thoughts?
I agree with what Stefan wrote above. It's a local issue only, and the complex interactions and backwards compatibility concerns call for public review, IMHO. And as I wrote, this has been documented as a source of security issues as early as 1996. I agree it's time to fix this in a central place, but where that should happen is not quite clear to me. I'm leaning towards glibc, but I'm biased. Thanks, Florian
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 2.1k
On Thu Jan 21 10:22:44 2016, fw@deneb.enyo.de wrote: Show quoted text
> * Red Hat Product Security: >
> > In order to keep things moving: Red Hat has assigned CVE-2016-0748 > > to a fix planned to be in glibc. I expect the libc fix to be some > > kind of a "milestone", which would allow us to go ahead and poke at > > all the other stuff that may need separate fixing. > > > > However, this appears to be one of the issues where it's very > > important to have a general consensus on how exactly the fix should > > work and look like. I do believe that finding this consensus will be > > a huge part of the work that needs to be done to resolve this issue. > > > > I don't think that what we have now, a collection of CCed parties, > > is a suitable place for discussing this. Thus, I think we should > > ultimately move this discussion to a more suitable place, such as a > > public mailing list. libc-alpha was suggested before. > > > > Thoughts?
> > I agree with what Stefan wrote above. It's a local issue only, and > the complex interactions and backwards compatibility concerns call for > public review, IMHO. > > And as I wrote, this has been documented as a source of security > issues as early as 1996. I agree it's time to fix this in a central > place, but where that should happen is not quite clear to me. I'm > leaning towards glibc, but I'm biased.
glibc (and correspondingly, the libc-alpha mailing list) specific discussion might not be directly relevant (*) to the BSD folks (I see at least Todd Miller on the CC list, and @mirbsd) Anyone here from FreeBSD? NetBSD? Or Apple? (*) Though, of course, while any particular patches for glibc will not directly applicable to non-glibc lands, in spirit and intent they might be useful. Maybe there needs to be three different "forks" of this discussion: the glibc, the *BSD camp, and then the shells/perl. (Non-UNIX environments, which at least Perl covers, are then yet another dimension, each their own.) But until there's some sort of general overall plan and rough consensus (anyone care to attempt a summary?), such forking of discussion is probably premature. Show quoted text
> Thanks, > Florian >
Date: Tue, 26 Jan 2016 00:08:51 -0500
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
CC: Red Hat Product Security <secalert [...] redhat.com>, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Download (untitled) / with headers
text/plain 1.3k
* Stephane CHAZELAS <stephane.chazelas@gmail.com> [2016-01-21T12:39:08] Show quoted text
> perl is possibly the most severely impacted (bypass of taint > mode) and runs on a variety of Unix-likes and non-Unix-likes. > > From the discussion here, they've discussed changing their %ENV > so it picks the first entry like getenv(3) on "get" and remove > duplicates on "set".
Yes, and I think we need to apply that. Also, because this affects the taint system, a Perl-specific feature meant to protect you from an untrusted environment, I think this goes beyond the well-known (I am told :)) problem of an ambiguous environment. I will be assigning the Perl-specific part of this problem a CVE number and doing pre-disclosure of the issue to our downstream packagers in the next few days, probably targeting Feb 9 for release. Tony Cook is producing an updated patch, which I'll be distributing downstream. Show quoted text
> If anybody has any objection to the issue being made public, > they should raise it now.
Although I wouldn't object to the immediate discussion of the problem in general, it is almost certainly going to lead to scrutiny of things that misbehave in the context of an ambiguous environment, meaning perl. I would prefer if the public discussion could wait until then. Thanks for your patience in waiting for a reply. I have been neglecting my inbox whilst my hometown was smothered in an absurd amount of snow. -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

CC: rt-deliver-to-perl5-security-report [...] rt.perl.org, Tony Cook <tony [...] develop-help.com>
From: "Craig A. Berry" <craig.a.berry [...] gmail.com>
To: bugs-comment [...] bugs6.perl.org
Date: Tue, 26 Jan 2016 07:22:45 -0600
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
Download (untitled) / with headers
text/plain 1.1k
On Mon, Jan 18, 2016 at 12:47 PM, Craig A. Berry via RT <bugs-comment@bugs6.perl.org> wrote: Show quoted text
> as far as handling duplicates in the C run-time's environ array, nothing special needs to be > done for VMS.
I've done further testing and now think the parts of S_init_postdump_symbols that we skip on VMS need to stay skipped for now. Which means Tony's first patch needs to be augmented by the attached patch to the equivalent bits in vms/vms.c. I will try to remember to apply it after the public disclosure, but if it could be simply added to the proposed patch circulated with the CVE, that would be great. --- vms/vms.c;-0 2016-01-16 18:36:16 -0600 +++ vms/vms.c 2016-01-21 15:51:27 -0600 @@ -1307,7 +1307,9 @@ prime_env_iter(void) if (!str$case_blind_compare(env_tables[i],&crtlenv)) { char *start; int j; - for (j = 0; environ[j]; j++) { + /* Start at the end, so if there is a duplicate we keep the first one. */ + for (j = 0; environ[j]; j++); + for (j--; j >= 0; j--) { if (!(start = strchr(environ[j],'='))) { if (ckWARN(WARN_INTERNAL)) Perl_warner(aTHX_ packWARN(WARN_INTERNAL),"Ill-formed CRTL environ value \"%s\"\n",environ[j]);

Message body is not shown because sender requested not to inline it.

Date: Tue, 26 Jan 2016 17:27:02 +0000
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
CC: chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
To: Red Hat Product Security <secalert [...] redhat.com>
Download (untitled) / with headers
text/plain 1.8k
2016-01-21 12:08:38 -0500, Red Hat Product Security: [...] Show quoted text
> In order to keep things moving: Red Hat has assigned CVE-2016-0748 to a fix > planned to be in glibc. I expect the libc fix to be some kind of a "milestone", > which would allow us to go ahead and poke at all the other stuff that may need > separate fixing. > > However, this appears to be one of the issues where it's very important to have > a general consensus on how exactly the fix should work and look like. I do > believe that finding this consensus will be a huge part of the work that needs > to be done to resolve this issue.
[...] Also note that doing it at the libc level means - doing it for every libc. On Linux alone, there are at least 3 that I know, and there's one more per other system. - recompiling static binaries (at least the setuid/setgid... ones that may execute commands). - could break things, so maybe should be done only for setuid/setgids which means shells would still potentially have a problem. Also, if perl fixes it with a CVE, it's going to become very obvious that bash, and BSDs "sh"s have the same issue. BTW, I've just found out that even though ksh93 is not affected, ksh88 seems to be (at least /usr/xpg4/bin/sh on Solaris). The Bourne shell may be as well, at least the Heirloom toolchest one (would affect Solaris 10 and earlier /bin/sh). I'm under the impression that fixing bash and BSDs sh (and perl which is already under way) is easier from that point of view. A deduplicating at the libc/kernel level would still be a good idea, as a hardening measure in case there still are applications that do use envp[] as a list of variables but query/set environment variables in a different way from getenv/putenv/setenv. I suspect it may be difficult to get a consensus among Unix vendors on the scope of that and how it should be done. -- Stephane
Date: Tue, 26 Jan 2016 17:41:55 +0000 (UTC)
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
To: "stephane.chazelas [...] gmail.com via RT" <perl5-security-report [...] perl.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
stephane.chazelas@gmail.com via RT dixit: Show quoted text
>Also note that doing it at the libc level means > - doing it for every libc. On Linux alone, there are at least 3 > that I know, and there's one more per other system.
Linux: • libc5 (yes I know) • glibc ‣ they said something about doing it in ld.so which would break for static binaries, so they’d have to do it twice ‣ OTOH this would fix Hurd and kFreeBSD in one go • musl • klibc • dietlibc • µClibc • µClibc-ng • bionic • the one I’m writing… (not currently officially published) Hurd: I only know of glibc … plus at least one libc per BSD, but they also have one kernel each… though fixing it in the FreeBSD kernel would also fix kFreeBSD in the same go. I’d *really* prefer this to be fixed in the various kernels (although, as userspace is allowed to change what environ points to, the libc may need to do that on reinitialisation… but only if we decide that’s an attack vector; it may not be since normalisation in the kernel happens at execve time). Now my own stance: BSD OS developer PoV: I’m likely to fix it in the kernel if I manage to do that (admittedly my kernel development skills are subpar), and possibly add something to libc as its *env() functions are severely buggy anyway. Although, I’m still undecided if kernel normalisation will use the first or the last occurrence (not looking at mksh, both make sense) and open to arguments for either from others (especially the OpenBSD people, as they have sorta the same code (modulo a couple of years…) and they’re usually technically right). mksh tool PoV: I’m not going to change mksh; it will always continue to use the *last* occurrence as this is a requirement for re-entrancy of environment dump/import. bye, //mirabilos -- Stéphane, I actually don’t block Googlemail, they’re just too utterly stupid to successfully deliver to me (or anyone else using Greylisting and not whitelisting their ranges). Same for a few other providers such as Hotmail. Some spammers (Yahoo) I do block.
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org
To: Thorsten Glaser via RT <perl5-security-report [...] perl.org>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Date: Tue, 26 Jan 2016 17:57:49 +0000
Download (untitled) / with headers
text/plain 824b
2016-01-26 09:46:48 -0800, Thorsten Glaser via RT: [...] Show quoted text
> mksh tool PoV: I’m not going to change mksh; it will always > continue to use the *last* occurrence as this is a requirement > for re-entrancy of environment dump/import.
[...] I'm not sure I understand that argument. mksh deduplicates the environment already. So if you dump the environmnet from mksh, you'll see only one entry per variable (currently the last one, not the one that was sanitized by your caller in the security issue being discussed here). Importing that dump will set the same variable. If you change that to be the first one, still deduplicated, you fix the security issue, your dump will still have one entry, which will still set the same variable on import. Can you please clarify what would break if you changed mksh? -- Stephane
To: "stephane.chazelas [...] gmail.com via RT" <perl5-security-report [...] perl.org>
From: Thorsten Glaser <tg [...] mirbsd.de>
CC: krahmer [...] suse.com, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
Date: Tue, 26 Jan 2016 18:15:53 +0000 (UTC)
Download (untitled) / with headers
text/plain 611b
stephane.chazelas@gmail.com via RT dixit: Show quoted text
>So if you dump the environmnet from mksh, you'll see only one
Yes, but you can dump it from a called program instead. Show quoted text
>Can you please clarify what would break if you changed mksh?
Consistency – the notation of the environment as a set of key=value assignments read in order. bye, //mirabilos -- Stéphane, I actually don’t block Googlemail, they’re just too utterly stupid to successfully deliver to me (or anyone else using Greylisting and not whitelisting their ranges). Same for a few other providers such as Hotmail. Some spammers (Yahoo) I do block.
Date: Tue, 26 Jan 2016 13:45:07 -0500
From: Jarkko Hietaniemi <jhi [...] iki.fi>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>, Red Hat Product Security <secalert [...] redhat.com>
CC: chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Download (untitled) / with headers
text/plain 737b
Show quoted text
> A deduplicating at the libc/kernel level would still be a good > idea, as a hardening measure in case there still are > applications that do use envp[] as a list of variables but > query/set environment variables in a different way from > getenv/putenv/setenv.
I think we need to think of defense in depth here: libc/kernel *and* the perl/shells. It's not like every system will want to / will be able to / will ever upgrade to a newer libc. With the number of libc/kernel combinations just across Linux and *BSD, never mind other UNIXy systems, doubly never mind non-UNIXy systems, it is almost guaranteed that some of those combinations will not be patched / will be patched differently / will be patched but somewhat later.
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 668b
On Mon Jan 25 21:09:09 2016, perl.security@rjbs.manxome.org wrote: Show quoted text
> Tony Cook is producing an updated patch, which I'll be distributing > downstream.
It's not so much an updated patch, but a new patch. This uses a different approach, rather than de-dupping a given name in environ[] on my_setenv() it scans for duplicates when we load %ENV, and if it finds any, removes them. Unlike the previous patch, where every use of my_setenv() paid a cost for the possibility of duplicate entries, for this patch there's (mostly) only a cost when duplicate entries are found. ("mostly" because there's an extra hv_exists() call perl entry on the non-duplicates path.) Tony
Subject: 0001-perl-127158-remove-duplicate-environment-variables-f.patch
From 8a5c18363465c8ce7477dfa604afad6be02d8726 Mon Sep 17 00:00:00 2001 From: Tony Cook <tony@develop-help.com> Date: Wed, 27 Jan 2016 11:52:15 +1100 Subject: [perl #127158] remove duplicate environment variables from environ If we see duplicate environment variables while iterating over environ[]: a) make sure we use the same value in %ENV that getenv() returns. Previously on a duplicate, %ENV would have the last entry for the name from environ[], but a typical getenv() would return the first entry. Rather than assuming all getenv() implementations return the first entry explicitly call getenv() to ensure they agree. b) remove duplicate entries from environ Previously if there was a duplicate definition for a name in environ[] setting that name in %ENV could result in an unsafe value being passed to a child process, so ensure environ[] has no duplicates. --- perl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 49 insertions(+), 2 deletions(-) diff --git a/perl.c b/perl.c index c7c1fe6..f33222b 100644 --- a/perl.c +++ b/perl.c @@ -4320,23 +4320,70 @@ S_init_postdump_symbols(pTHX_ int argc, char **argv, char **env) } if (env) { char *s, *old_var; + STRLEN nlen; SV *sv; + HV *dups = newHV(); + for (; *env; env++) { old_var = *env; if (!(s = strchr(old_var,'=')) || s == old_var) continue; + nlen = s - old_var; #if defined(MSDOS) && !defined(DJGPP) *s = '\0'; (void)strupr(old_var); *s = '='; #endif - sv = newSVpv(s+1, 0); - (void)hv_store(hv, old_var, s - old_var, sv, 0); + if (hv_exists(hv, old_var, nlen)) { + const char *name = savepvn(old_var, nlen); + + /* make sure we use the same value as getenv(), otherwise code that + uses getenv() (like setlocale()) might see a different value to %ENV + */ + sv = newSVpv(PerlEnv_getenv(name), 0); + + /* keep a count of the dups of this name so we can de-dup environ later */ + if (hv_exists(dups, name, nlen)) + ++SvIVX(*hv_fetch(dups, name, nlen, 0)); + else + (void)hv_store(dups, name, nlen, newSViv(1), 0); + + Safefree(name); + } + else { + sv = newSVpv(s+1, 0); + } + (void)hv_store(hv, old_var, nlen, sv, 0); if (env_is_not_environ) mg_set(sv); } + if (HvKEYS(dups)) { + /* environ has some duplicate definitions, remove them */ + HE *entry; + hv_iterinit(dups); + while ((entry = hv_iternext_flags(dups, 0))) { + STRLEN nlen; + const char *name = HePV(entry, nlen); + IV count = SvIV(HeVAL(entry)); + IV i; + SV **valp = hv_fetch(hv, name, nlen, 0); + + assert(valp); + + /* try to remove any duplicate names, depending on the + * implementation used in my_setenv() the iteration might + * not be necessary, but let's be safe. + */ + for (i = 0; i < count; ++i) + my_setenv(name, 0); + + /* and set it back to the value we set $ENV{name} to */ + my_setenv(name, SvPV_nolen(*valp)); + } + } + SvREFCNT_dec_NN(dups); } #endif /* USE_ENVIRON_ARRAY */ #endif /* !PERL_MICRO */ -- 2.1.4
Date: Tue, 26 Jan 2016 22:46:54 -0500
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
To: Jarkko Hietaniemi <jhi [...] iki.fi>
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
CC: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>, Red Hat Product Security <secalert [...] redhat.com>, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Download (untitled) / with headers
text/plain 701b
* Jarkko Hietaniemi <jhi@iki.fi> [2016-01-26T13:45:07] Show quoted text
> >A deduplicating at the libc/kernel level would still be a good > >idea, as a hardening measure in case there still are > >applications that do use envp[] as a list of variables but > >query/set environment variables in a different way from > >getenv/putenv/setenv.
> > I think we need to think of defense in depth here: libc/kernel *and* the > perl/shells.
This is my position as well. Of course I would like everyone to have all the latest patches for libc and kernel problems, but I don't think this is a situation where we can simply say "go do that," especially because of the enormous number of kernels and libcs we support! -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

Date: Tue, 26 Jan 2016 22:57:43 -0500
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
CC: Red Hat Product Security <secalert [...] redhat.com>, chet.ramey [...] case.edu, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Download (untitled) / with headers
text/plain 553b
* Stephane CHAZELAS <stephane.chazelas@gmail.com> [2016-01-26T12:27:02] Show quoted text
> Also, if perl fixes it with a CVE, it's going to become very > obvious that bash, and BSDs "sh"s have the same issue.
Yes. I suppose we can try to coordinate at least *some* of the many affected things, but is this a fool's errand? Chet, would you, at any rate, like to do that? I have not yet started notification for this problem, as we just got another taint-related ticket today and I'd like to combine them if we agree with the other report. Tomorrow, maybe. -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

CC: chet.ramey [...] case.edu, Red Hat Product Security <secalert [...] redhat.com>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
To: Ricardo Signes <perl.security [...] rjbs.manxome.org>, Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
From: Chet Ramey <chet.ramey [...] case.edu>
Date: Wed, 27 Jan 2016 11:41:34 -0500
Download (untitled) / with headers
text/plain 1.6k
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 1/26/16 10:57 PM, Ricardo Signes wrote: Show quoted text
> * Stephane CHAZELAS <stephane.chazelas@gmail.com> [2016-01-26T12:27:02]
>> Also, if perl fixes it with a CVE, it's going to become very >> obvious that bash, and BSDs "sh"s have the same issue.
> > Yes. I suppose we can try to coordinate at least *some* of the many affe
cted Show quoted text
> things, but is this a fool's errand? > > Chet, would you, at any rate, like to do that?
I'm not enthusiastic about any of the proposed `solutions'. Stephane's statement could just as easily apply to any of the C libraries on any affected system, or any program that manipulates its environment without relying on getenv/setenv/putenv. I think the right place to apply any fix is in libc. I'm sympathetic to Thorsten's argument that the environment is an ordered set of name-value pairs and there is no reason to prefer the first instance of a particular name other than the (fairly weak) argument that "getenv/setenv do it that way." There's a consistency in treating something like "a=1 a=2 a=3 prog" identically to `prog' receiving an environment with [0]='a=1' [1]='a=2' [2]='a=3'. The question is whether `fixing' shells and other programs to behave like getenv and leaving the root cause (at least temporarily) unaddressed will do any good. Chet - -- ``The lyf so short, the craft so long to lerne.'' - Chaucer ``Ars longa, vita brevis'' - Hippocrates Chet Ramey, ITS, CWRU chet@case.edu http://cnswww.cns.cwru.edu/~chet/ -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iEYEARECAAYFAlao8zQACgkQu1hp8GTqdKvIeACfcaZhMJrrEIHkAxSlNooz/e01 f4wAmwam6Wgnc1OoVm2/0d3rS4JfMC5q =tzf0 -----END PGP SIGNATURE-----
Date: Wed, 27 Jan 2016 17:20:17 +0000
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
To: Chet Ramey <chet.ramey [...] case.edu>
CC: Ricardo Signes <perl.security [...] rjbs.manxome.org>, Red Hat Product Security <secalert [...] redhat.com>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Download (untitled) / with headers
text/plain 1.7k
2016-01-27 11:41:34 -0500, Chet Ramey: [...] Show quoted text
> I'm sympathetic to Thorsten's argument that the environment is an ordered > set of name-value pairs and there is no reason to prefer the first > instance of a particular name other than the (fairly weak) argument that > "getenv/setenv do it that way."
The argument is that if we do it differently from getenv/setenv, then we're introducing the security vulnerability described here. perl's latest approach to actually call getenv to assign %ENV elements would avoid the question of which order to scan the environment. Show quoted text
> There's a consistency in treating > something like "a=1 a=2 a=3 prog" identically to `prog' receiving an > environment with [0]='a=1' [1]='a=2' [2]='a=3'.
both of which are pathological cases. (And for the first case, several historic shell implementations have performed those assignments right to left, "a=1 a=2 a=3 printenv a" still prints "a" with Solaris 10 /bin/sh for instance) Again changing bash won't break anything. Two env variables with the same name is a pathological case, it should not happen in real life, the only reason to do it would be to try and exploit this vulnerability. Show quoted text
> The question is whether `fixing' shells and other programs to behave like > getenv and leaving the root cause (at least temporarily) unaddressed will > do any good.
[...] One consideration is that "fixing" bash once now will fix it wherever bash is deployed. Fixing the "root cause" involves changing a lot of different software and may never happen on some systems and I can't see it happen before perl publishes their fix. I agree we should fix the root case, but "fixing" bash won't do any harm (note that ksh93 and zsh already do it) and will definitely help. -- Stephane
Date: Wed, 27 Jan 2016 20:57:15 -0500
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
To: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
CC: Chet Ramey <chet.ramey [...] case.edu>, Red Hat Product Security <secalert [...] redhat.com>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
Download (untitled) / with headers
text/plain 1.1k
* Stephane CHAZELAS <stephane.chazelas@gmail.com> [2016-01-27T12:20:17] Show quoted text
> 2016-01-27 11:41:34 -0500, Chet Ramey:
> > The question is whether `fixing' shells and other programs to behave like > > getenv and leaving the root cause (at least temporarily) unaddressed will > > do any good.
> [...] > > One consideration is that "fixing" bash once now will fix it > wherever bash is deployed. Fixing the "root cause" involves > changing a lot of different software and may never happen on > some systems and I can't see it happen before perl publishes > their fix.
I agree with Stephane here, but let me put that agreement aside for a moment: perl is affected in a way that other pieces of software are not, because of its taint system. It's not reasonable for us to leave that broken while waiting for every libc to be fixed, so we're going to have to fix that. When we do, presumably, it will draw some attention to the problem space. So, perl has to make a change. We can hold off a bit on making the change in case others would like to synchronize. If everyone else is content to wait for libc fixes, though, we're not going to wait indefinitely. -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

Date: Thu, 28 Jan 2016 10:44:43 +0000
From: Stephane CHAZELAS <stephane.chazelas [...] gmail.com>
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
CC: Ricardo Signes <perl.security [...] rjbs.manxome.org>, Red Hat Product Security <secalert [...] redhat.com>, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, perl5-security-report [...] perl.org, security [...] suse.de, team [...] security.debian.org
To: Chet Ramey <chet.ramey [...] case.edu>
Download (untitled) / with headers
text/plain 570b
2016-01-27 17:20:17 +0000, Stephane CHAZELAS: [...] Show quoted text
> I agree we should fix the root case, but "fixing" bash won't do > any harm (note that ksh93 and zsh already do it) and will > definitely help.
[...] For the record, it would seem the "issue" was "fixed" (though it doesn't appear the fix specifically addressed that vulnerability) in zsh in 1997 in 3.1.3 (3.1.2-zefram1) with this commit: http://www.zsh.org/mla/workers/1997/msg00555.html (while importing variables from the environment, don't import variables that are already marked as exported). -- Stephane
From: Stephane Chazelas <stephane.chazelas [...] gmail.com>
To: Chet Ramey <chet.ramey [...] case.edu>
CC: Ricardo Signes <perl.security [...] rjbs.manxome.org>, Red Hat Product Security <secalert [...] redhat.com>, ridiculous_fish <corydoras [...] ridiculousfish.com>, Herbert Xu <herbert [...] gondor.apana.org.au>, Magicant <magicant [...] users.osdn.me>, Tony Cook via RT <perl5-security-report [...] perl.org>, security [...] suse.de, team [...] security.debian.org
Subject: Re: [engineering.redhat.com #385443] security issue with multiple environment entries with the same name [perl #127158]
Date: Thu, 28 Jan 2016 13:41:32 +0000
Download (untitled) / with headers
text/plain 803b
On 28 January 2016 at 10:44, Stephane CHAZELAS <stephane.chazelas@gmail.com> wrote: Show quoted text
> For the record, it would seem the "issue" was "fixed" (though it > doesn't appear the fix specifically addressed that > vulnerability) in zsh in 1997 in 3.1.3 (3.1.2-zefram1) with this > commit: > > http://www.zsh.org/mla/workers/1997/msg00555.html > > (while importing variables from the environment, don't import > variables that are already marked as exported).
[...] Sorry, my bad. That commit actually did mention that it was doing the same thing as ksh at the time and picking the second env var when there are duplicates. It was actually fixed by this commit: http://www.zsh.org/mla/workers/2000/msg03237.html (4.0 (3.1.9-dev-4) in 2000) (again not specifically to address the vulnerability) -- Stephane
To: Tony Cook via RT <perl5-security-report [...] perl.org>
From: Ricardo Signes <perl.security [...] rjbs.manxome.org>
Date: Sat, 13 Feb 2016 09:24:10 -0500
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: ;, rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 677b
* Tony Cook via RT <perl5-security-report@perl.org> [2016-01-26T20:04:03] Show quoted text
> On Mon Jan 25 21:09:09 2016, perl.security@rjbs.manxome.org wrote:
> > Tony Cook is producing an updated patch, which I'll be distributing > > downstream.
> > It's not so much an updated patch, but a new patch.
This got a bit derailled by a failed attempt to coordinate disclosure. Nothing has come of that, so I'm moving forward. Perl security team: I just pushed branch "env-dupes" to the security repository. I will send that downstream with an embargo on Tuesday if I don't get objections. Craig: this also includes your vms.c patch. I made matching branches for 5.22 and 5.20. -- rjbs
Download signature.asc
application/pgp-signature 473b

Message body not shown because it is not plain text.

RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
The Perl ENV problems have been assigned CVE-2016-2381. -- rjbs
Subject: Re: [perl #127158] [engineering.redhat.com #385443] security issue with multiple environment entries with the same name
CC: Thorsten Glaser <tg [...] mirbsd.de>, Jarkko Hietaniemi via RT <perl5-security-report [...] perl.org>, krahmer [...] suse.com, stephane.chazelas [...] gmail.com, corydoras [...] ridiculousfish.com, herbert [...] gondor.apana.org.au, magicant [...] users.osdn.me, secalert [...] redhat.com, security [...] suse.de, team [...] security.debian.org, tg+security [...] mirbsd.org
To: Chet Ramey <chet.ramey [...] case.edu>
From: Florian Weimer <fw [...] deneb.enyo.de>
Date: Tue, 01 Mar 2016 16:46:47 +0100
Download (untitled) / with headers
text/plain 559b
* Florian Weimer: Show quoted text
> * Chet Ramey: >
>> I agree that this is something that should be fixed in the C library or the >> kernel.
> > I agree. I would welcome a discussion on libc-alpha. Any fix will > have to be subject to public review because it's a pretty significant > change. One problem is that we have to implement very early in > process startup in glibc, where nothing of the library proper is > available for use.
Now that the issue is public, I filed a glibc bug for it: <https://sourceware.org/bugzilla/show_bug.cgi?id=19749> Thanks, Florian
RT-Send-CC: rt-deliver-to-perl5-security-report [...] rt.perl.org
Download (untitled) / with headers
text/plain 212b
On Tue Mar 01 07:47:40 2016, fw@deneb.enyo.de wrote: Show quoted text
> Now that the issue is public, I filed a glibc bug for it:
Is there anything that's been discussed in this thread that anyone doesn't want public yet? Tony
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 276b
On Wed Mar 02 15:19:13 2016, tonyc wrote: Show quoted text
> On Tue Mar 01 07:47:40 2016, fw@deneb.enyo.de wrote:
> > Now that the issue is public, I filed a glibc bug for it:
> > Is there anything that's been discussed in this thread that anyone > doesn't want public yet?
Now public. Tony
Download (untitled) / with headers
text/plain 252b
Thank you for submitting this report. You have helped make Perl better. With the release of Perl 5.24.0 on May 9, 2016, this and 149 other issues have been resolved. Perl 5.24.0 may be downloaded via https://metacpan.org/release/RJBS/perl-5.24.0


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org