Re: Fwd: BadBunny: UFFDIO_COPY shmem killpriv bypass leading to local privilege escalation

On Tue 11-08-26 14:16:31, vova tokarev wrote:
> Matthew, Christian, Pedro,
> 
> Lol - fair point, I'll keep the reggaeton references out of future
> commit messages.
> 
> Agreed on severity - but this is a
> killpriv invariant violation, and the kernel has treated those as
> security fixes worth backporting before.

I tend to agree this would be good to fix for consistency as much as I also
don't think this is a particularly serious issue. Removing SUID bits on
write is more of a hardening measure than serious security guarantee and is
there mostly for historical reasons AFAIK from the old Unix days.  POSIX
for write(2) mentions this as: "Upon successful completion, where nbyte is
greater than 0, write() shall mark for update the st_ctime and st_mtime
fields of the file, and if the file is a regular file, the S_ISUID and
S_ISGID bits of the file mode may be cleared." truncate(), chmod(), chown()
have similar notes. No mention of this when handling mmap BTW :).

Anyway, the problem with calling __file_remove_privs() when writing through
mmap is with the implementation - in particular the locking. To be able to
call __file_remove_privs() we need to hold i_rwsem and that ranks above VMA
locks / mmap_lock we hold during the page fault. Now that I'm speaking
about it I even have a vague recollection this has already come up in the
past and we've just decided to leave it alone due to these technical
difficulties.

> 1. The fallocate killpriv fixes (XFS fbe7e5200365, ext4, f2fs) are
>    exact precedent: same reasoning ("can change the file contents [...]
>    should drop file privileges like suid just like we do for a regular
>    write()"), same one-line fix, and they went through the security fix
>    process with CVE assignment and stable backports.
> 
> 2. CVE-2023-0386 (overlayfs SUID preservation) -- same bug class,
>    CVSS 7.8, CISA KEV.
> 
> 3. If permissions alone protected SUID, write() wouldn't strip it.
>    killpriv exists for POSIX ACLs granting write to non-owners,
>    group-writable SUID, container shared mounts, and chaining with
>    other write-access bugs.
> 
> 4. This path is reachable unprivileged even with
>    vm.unprivileged_userfaultfd=0 (UFFD_USER_MODE_ONLY bypasses it).
> 
> 5. Pedro's point that MAP_SHARED faults also skip killpriv isn't a
>    counterargument -- it's another instance of the same class. We can
>    fix them independently.
> 
> Given that the fallocate killpriv fixes went through the security fix
> process (CVE + stable backport), should this follow the same path?
> Happy to send the patch either way.

I didn't see the patch, maybe there the locking could be reconciled in an
easier way (although I have some doubts ;)). Anyway, without solving the
locking issues I don't think we should fix this.

								Honza

-- 
Jan Kara <[email protected]>
SUSE Labs, CR