Re: [PATCH] pt_chown: Clear any signal mask inherited from the parent process.

[Geoffrey Thomas <geofft at ldpreload dot com>]

On Mon, 18 May 2015, Mike Frysinger wrote:

> > > also, the code needs to be audited to make sure that sending arbitrary signals
> > > can't be abused to make it skip security checks or leave things in a bad state.
> > > rather than unmask all, it might want to unmask one and make sure that one
> > > results in its immediate death.
> >
> > Signal handlers get reset on exec, so the only thing a signal can do is
> > immediately kill the process, suspend it, or do nothing, all of which are
> > fine. The worst thing you can do is kill it between chown and chmod, but
> > if pt_chown exits successfully, the unprivileged user owns the tty anyway
> > and can chmod it however they like.
>
> that doesn't mean the code is written to assume the user can send it arbitrary
> signals like either kill it at the wrong place or STOP/CONT that can interrupt
> some system calls.

I'm not sure I follow. It's _already_ true that the user can send pt_chown 
arbitrary signals, since it does not do anything to either mask or unmask 
signals. If there's some such vulnerability in pt_chown, I can already 
send it SIGINT, SIGTSTP, SIGQUIT, or SIGHUP by running it in a terminal, 
or SIGALRM by calling alarm() before execve(), or SIGKILL by setting a CPU 
limit, or lots of other things.

The only thing this patch does is make the execution environment 
consistent, regardless of what the signal mask is of the thread calling 
grantpt(). (It's also possible to put this code in the implementation of 
grantpt(), but fixing it post-exec is more strightforward, since you don't 
have to worry about signal handlers.) If being able to signal pt_chown is 
a problem, it's _already_ a problem, and it's not made worse by this 
patch.

If you're worried that pt_chown is not robust to being sent signals, then 
it should explicitly mask or ignore signals while it's running; I'm happy 
to send in a patch to do that. Or we could just get rid of it as you 
suggested, which would also be totally fine with me.

--
Geoffrey Thomas
https://ldpreload.com
geofft@ldpreload.com