Linux sandbox: save full list of SUID unsafe environment variables.

Linux sandbox: save full list of SUID unsafe environment variables.

r20733 added code to save LD_LIBRARY_PATH when using the SUID sandbox.
That fixed a P0, show-stopper bug, however, LD_LIBRARY_PATH isn't the
only variable which is stomped when using SUID binaries. This patch
extends support to all variables that we so affected.

BUG=16815

[fta, 2009-07-17 20:58:15]

what about GTK_PATH? it's used as a workaround on x64 for gtk2 unable to load
modules from /usr/lib32 (using ia32libs)

[Evan Martin, 2009-07-17 21:10:57]

fta: we pass through all variables implicitly since they inherit the parent
process's environment.  This is just passing through the limited set of vars
that ld.so eats because we have a setuid binary in the middle.

http://codereview.chromium.org/159025/diff/6/1005
File chrome/browser/zygote_host_linux.cc (right):

http://codereview.chromium.org/159025/diff/6/1005#newcode42
Line 42: unsetenv(saved_envvar);
I wonder if we should unsetenv() all existing SANDBOX_FOO here so that any extra
ones from the env don't get used by the child.  Maybe that's excessive.

http://codereview.chromium.org/159025/diff/6/1006
File sandbox/linux/suid/sandbox.cc (right):

http://codereview.chromium.org/159025/diff/6/1006#newcode233
Line 233: // ld.so may have cleared several environment variable because we are
SUID.
variable*s*

http://codereview.chromium.org/159025/diff/6/1007
File sandbox/linux/suid/suid_unsafe_environment_variables.h (right):

http://codereview.chromium.org/159025/diff/6/1007#newcode49
Line 49: 8 /* strlen("SANDBOX_") */;
This seems like a ton of effort to go through.  Why not just
  return std::string("SANDBOX_") + envvar;

Eliminates manual memory management on the callee side oto.

[agl, 2009-07-17 21:35:09]

http://codereview.chromium.org/159025/diff/6/1005
File chrome/browser/zygote_host_linux.cc (right):

http://codereview.chromium.org/159025/diff/6/1005#newcode42
Line 42: unsetenv(saved_envvar);
On 2009/07/17 21:10:57, Evan Martin wrote:
> I wonder if we should unsetenv() all existing SANDBOX_FOO here so that any
extra
> ones from the env don't get used by the child.  Maybe that's excessive.

We already control all the SANDBOX_ values: either we set them ourselves or we
clear them.

http://codereview.chromium.org/159025/diff/6/1006
File sandbox/linux/suid/sandbox.cc (right):

http://codereview.chromium.org/159025/diff/6/1006#newcode233
Line 233: // ld.so may have cleared several environment variable because we are
SUID.
On 2009/07/17 21:10:57, Evan Martin wrote:
> variable*s*

Done.

http://codereview.chromium.org/159025/diff/6/1007
File sandbox/linux/suid/suid_unsafe_environment_variables.h (right):

http://codereview.chromium.org/159025/diff/6/1007#newcode49
Line 49: 8 /* strlen("SANDBOX_") */;
On 2009/07/17 21:10:57, Evan Martin wrote:
> This seems like a ton of effort to go through.  Why not just
>   return std::string("SANDBOX_") + envvar;
> 
> Eliminates manual memory management on the callee side oto.

I'm currently sticking to the idea that the sandbox is pure-C99 code (although
we do compile as C++ because our build system is setup that way).