• Kees Cook's avatar
    exec: Fix ToCToU between perm check and set-uid/gid usage · f50733b4
    Kees Cook authored
    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.
    Reported-by: default avatarMarco Vanotti <mvanotti@google.com>
    Tested-by: default avatarMarco Vanotti <mvanotti@google.com>
    Suggested-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    Cc: stable@vger.kernel.org
    Cc: Eric Biederman <ebiederm@xmission.com>
    Cc: Alexander Viro <viro@zeniv.linux.org.uk>
    Cc: Christian Brauner <brauner@kernel.org>
    Signed-off-by: default avatarKees Cook <kees@kernel.org>
    f50733b4
exec.c 53.6 KB