Re: [musl] Calling setxid() in a vfork()-child

[Alexey Izbyshev <izbyshev@ispras.ru>]

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