[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <fcfe55b416f8225de3875699bc126250@ispras.ru>
Date: Mon, 12 Oct 2020 23:30:58 +0300
From: Alexey Izbyshev <izbyshev@...ras.ru>
To: musl@...ts.openwall.com
Subject: Re: Calling setxid() in a vfork()-child
Thank you for the response, Rich!
On 2020-10-12 17:55, Rich Felker wrote:
> Note that in addition to the issue you're asking about, it's
> fundamentally a bad idea to be using set*id() in a vforked child (or
> anywhere in a process that calls vfork) because it leaves moments
> where there are tasks in different privilege domains executing from
> the same VM space. If the task that's dropped privileges does anything
> that could lead to an attacker seizing control of the flow of
> execution, rather than just getting access to the set*id()-reduced
> privilege domain, they have full access to the original privilege
> domain. This is why musl's multithreaded set*id() (__synccall) takes
> care not to admit forward progress of any application code during the
> transition, and goes to the trouble of having a thread list lock that
> unlocks atomically with kernel task exit so that there is no race
> window where a still-live thread can be missed.
>
Yes, I initially learned about this security issue from your post[1]. I
proposed to ignore it for the moment in my email because it was hard for
me to imagine any security implications in a constrained case: if the
only use of setuid() in a privileged app is to drop privileges in a
vfork()-child before calling execve(). However, thinking about it more,
I see that dropping privileges could open the child to new ways of
interaction from outside of the app in a window before execve(), so, if,
say, another unprivileged process can ptrace it at the right moment, bad
things could happen. But now I don't see why wouldn't the same attack be
applicable to __synccall(). While __synccall() seizes execution of
application code, I don't see how it could prevent ptrace-like kind of
an outside attack on a thread at the moment when another thread is in a
different privilege domain.
> In any case, IMO unless you're programming for NOMMU compatibility,
> you should just forget vfork ever existed. There's no good reason to
> use it. If a process can't fork because it's too big or the fork would
> impact performance too much, posix_spawn can do far more than
> vfork+execve can do portably. It can't do everything you can do with
> vfork+execve if you're willing to break portability rules (i.e. invoke
> UB), but with a helper executable to run in the child you can get that
> all back.
>
Unfortunately, while posix_spawn() + a helper executable would be fine
in many cases (more so for applications than libraries), addition of a
helper to an existing library exposing a process creation API that can't
be implemented in terms of posix_spawn() may be not straightforward. If
the helper is just an executable file, we'll need to locate it, which
may be simply impossible when our library is called if, say, switching
mount namespaces is involved (which may be out of control of our
library). Solutions may exist (locate and open() at startup or
memfd_create(), and then execveat()?), but vfork() (+ preventing
execution of signal handlers in the child) seems so much simpler, and
doesn't add the overhead of an extra execve() too.
Alexey
[1] https://ewontfix.com/7
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.
