[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20110822181334.GA132@brightrain.aerifal.cx>
Date: Mon, 22 Aug 2011 14:13:34 -0400
From: Rich Felker <dalias@...ifal.cx>
To: musl@...ts.openwall.com
Subject: Re: env vars in SUID/SGID programs (was: LD_PRELOAD and
RTLD_NEXT support)
On Mon, Aug 22, 2011 at 09:02:06PM +0400, Solar Designer wrote:
> Rich, Vasiliy -
>
> On Tue, Aug 16, 2011 at 04:46:00PM +0400, Vasiliy Kulikov wrote:
> > ...btw, I feel it would be cleaner if you check for untrusted environment
> > at the time of initializing env_* variables. Currently there is not
> > much code between env_X assignment and zeroing, but it might be in the
> > future (with addition of ld features, etc.).
> >
> > for (p = argv+i; ... ) {
> > if (is_secure_env)
> > env_path = ...
>
> I just took a look at recent musl, and I don't see any safety from env
> vars used elsewhere in musl.
>
> If a SUID/SGID program uses execvp(3), what do we want to happen?
>
> Also, for env vars that musl ignores because of SUID/SGID'ness of the
> current program, shouldn't it also unset them? This is what we're doing
> in glibc on Owl, because of sudo-like programs (where another program is
> invoked next - still privileged, but no longer detected as such by libc).
>
> I think these potential precautions I am talking about above should
> apply for both static and dynamic binaries. That is, they should be in
> musl's program startup code that is used in both cases.
The startup code (__libc_start_main) is the right place for this
check, and to merge nik'a vdso support for static-linked programs we
need auxv parsing here anyway, so it's minimal added cost. I just
haven't done it yet, partly for some silly micro-opt reasons that are
hard to explain -- basically there's some mildly ugly code I should
remove at the same time because there will no longer be any cases
where it could actually help, and I want to make sure that's cleaned
up right.
As for what specifically should be done, that's less clear to me.
Surely any use of environment vars (TMP?) in musl itself should be
conditional on !suid, but I don't see any allowance in the
specification for environment variables to "disappear" when an
suid/sgid program is exec'd. While unsetting them may help work around
some buggy suid programs that exec other programs without sanitizing
the environment, (1) it can't help with vars that libc doesn't know
about, and (2) it prevents legitimate, correct suid programs from
analyzing the environment and copying through or otherwise using some
security-related variables they might want to be able to see.
Especially for the normal suid behavior (just dropping privs and
running as the real uid after performing a single privileged
operation) it's desirable for the real user's environment (e.g. tmp
file location) to have effect.
Ultimately, suid programs that intend to invoke other programs which
aren't aware of having elevated privileges MUST perform the equivalent
of clearenv()/environ=0;, possibly after backing up the environment if
they need to examine it themselves... Anything else is dangerous,
whatever libc does.
Rich
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
