Re: [PATCH] Restrict access to TIOCLINUX

On Sun, Apr 02, 2023 at 04:08:15PM +0200, Hanno Böck wrote:
> Hi,
> 
> I'm sending this here before I'll try to send it to lkml and the
> respective maintainers to get some feedback first.
> 
> The TIOCLINUX functionality in the kernel can be abused for privilege
> escalation, similar to TIOCSTI. I considered a few options how to fix
> this, and this is what I came up with.
> 
> 
> Restrict access to TIOCLINUX
> 
> TIOCLINUX can be used for privilege escalation on virtual terminals when
> code is executed via tools like su/sudo.
> By abusing the selection features a lower-privileged application can
> write content to the console, select and copy/paste that content and
> thereby executing code on the privileged account. See also the poc here:
>   https://www.openwall.com/lists/oss-security/2023/03/14/3
> 
> Selection is usually used by tools like gpm that provide mouse features
> on the virtual console. gpm already runs as root (due to earlier
> changes that restrict access to a user on the current tty), therefore
> it will still work with this change.
> 
> The security problem mitigated is similar to the security risks caused
> by TIOCSTI, which, since kernel 6.2, can be disabled with
> CONFIG_LEGACY_TIOCSTI=n.
> 
> Signed-off-by: Hanno Böck <[email protected]>
> ---
>  drivers/tty/vt/vt.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 3c2ea9c098f7..3671173109b8 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3146,10 +3146,14 @@ int tioclinux(struct tty_struct *tty, unsigned
> long arg) switch (type)
>  	{
>  		case TIOCL_SETSEL:
> +			if (!capable(CAP_SYS_ADMIN))
> +				return -EPERM;

You just now broke any normal user programs that required this (or the
other ioctls), and so you are going to have to force them to be run with
CAP_SYS_ADMIN permissions?  That feels like you are going backwards in
security, not forwards.

And you didn't change anything for programs like gpm that already had
root permission (and shouldn't that permission be dropped anyway?)

thanks,

greg k-h