ACK: [SRU][N][PATCH 0/1] CVE-2024-43882

[ivanhu]

Acked-by: Ivan Hu <ivan.hu at canonical.com>


On 9/25/24 09:40, Koichiro Den wrote:
> [Impact]
> 
> exec: Fix ToCToU between perm check and set-uid/gid usage
> 
> When opening a file for exec via do_filp_open(), permission checking is
> done against the file's metadata at that moment, and on success, a file
> pointer is passed back. Much later in the execve() code path, the file
> metadata (specifically mode, uid, and gid) is used to determine if/how
> to set the uid and gid. However, those values may have changed since the
> permissions check, meaning the execution may gain unintended privileges.
> 
> For example, if a file could change permissions from executable and not
> set-id:
> 
> ---------x 1 root root 16048 Aug  7 13:16 target
> 
> to set-id and non-executable:
> 
> ---S------ 1 root root 16048 Aug  7 13:16 target
> 
> it is possible to gain root privileges when execution should have been
> disallowed.
> 
> While this race condition is rare in real-world scenarios, it has been
> observed (and proven exploitable) when package managers are updating
> the setuid bits of installed programs. Such files start with being
> world-executable but then are adjusted to be group-exec with a set-uid
> bit. For example, "chmod o-x,u+s target" makes "target" executable only
> by uid "root" and gid "cdrom", while also becoming setuid-root:
> 
> -rwxr-xr-x 1 root cdrom 16048 Aug  7 13:16 target
> 
> becomes:
> 
> -rwsr-xr-- 1 root cdrom 16048 Aug  7 13:16 target
> 
> But racing the chmod means users without group "cdrom" membership can
> get the permission to execute "target" just before the chmod, and when
> the chmod finishes, the exec reaches brpm_fill_uid(), and performs the
> setuid to root, violating the expressed authorization of "only cdrom
> group members can setuid to root".
> 
> Re-check that we still have execute permissions in case the metadata
> has changed. It would be better to keep a copy from the perm-check time,
> but until we can do that refactoring, the least-bad option is to do a
> full inode_permission() call (under inode lock). It is understood that
> this is safe against dead-locks, but hardly optimal.
> 
> [Fix]
> 
> Noble:  Clean cherry-pick
> Jammy:  fixed via stable
> Focal:  fixed via stable
> Bionic: fix sent to esm ML
> Xenial: fix sent to esm ML
> Trusty: won't fix
> 
> [Test Case]
> 
> Compile and boot tested
> 
> [Where problems could occur]
> 
> This fix addresses issues which is generic in nature, an issue with this
> fix would cause unexpected privilege escalation in rare race condition.
> 
> 
> Kees Cook (1):
>    exec: Fix ToCToU between perm check and set-uid/gid usage
> 
>   fs/exec.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
>