Potential security flaw of 9pfs: SUID/SGID not drop on file write in 9pfs

[ericvh]

Impact for 9p is basically the same as CVE-2018-13405, and the core
misbehaviour described was [1], quote:

"The intended behavior is that the non-member user can trigger creation of a
directory with group execution and SGID permission bits set whose group
ownership is of that group, but not a plain file."

[1] https://bugzilla.redhat.com/show_bug.cgi/?id=1599161

The thing here is though, this behaviour seems to differ among OSes and even
individual file systems: some would inherit permission bits to both newly
created directories and files, and others only for new directories. XFS
appears to have a config option fs.xfs.irix_sgid_inherit for that.

There is neither a specific behaviour restriction defined for either POSIX or
Linux that would cover the described expected behaviour, no?

+CC Dominique, Eric (9pfs Linux client)

On Wednesday, March 1, 2023 4:26:49 PM CET Michael S. Tsirkin wrote:
+CC Greg and Christian who know more about 9pfs.
Guys could you chime in on the security impact of this? Thanks!

On Wed, Mar 01, 2023 at 05:14:29PM +0800, JIETAO XIAO wrote:
Hi,

Our team finds when a local user in the guest tries to write an executable file
with SUID or SGID, none of these privileged bits are correctly dropped. As a
result, in rare circumstances(exist an executable file owned by root, writable
by others, has SUID/SGID bits), this flaw could be used by malicious users in
the guest to elevate their privileges within the guest and help a host's local
user to elevate privileges on the host.

Environment:

• QEMU version: QEMU emulator version 6.2.0
• Host/Guest architecture: X86
• Host kernel version: 5.17.0-rc1
• Guest kernel version: 6.1.14

Configurations:
We have tested it under different 9pfs configurations. The Qemu startup file is
like this:

sudo qemu-system-x86_64
-kernel /home/shawotao/tmp/linux/arch/x86/boot/bzImage
-append 'root=/dev/sda rw console=ttyS0'
-drive file=rootfs.img,format=raw
-M pc -cpu host --enable-kvm -smp 2
-m 2G
-chardev stdio,mux=on,id=mon -mon chardev=mon,mode=readline
-device virtio-serial-pci -device virtconsole,chardev=mon -vga none
-display none
-virtfs local,path=/tmp/test,mount_tag=myfs,security_model=passthrough,id=
fsdev0,writeout=immediate

Effected Configuration: fsdriver = local, all security model is affected,
mount 9pfs with cache=mmap

Reproduce Steps:

1. Use the above configuration to startup QEMU.
2. Mount 9pfs in the guest: root@virt: ~# mount -t 9p -o trans=virtio myfs /
   mnt -oversion=9p2000.L,posixacl,msize=512000,cache=mmap
3. Prepare an executable file owned by root, writable by others, and has SUID,
   SGID: root@virt:/mnt# ./prepare_suid_file.sh
4. Change to a local user: root@virt:/mnt# su shawtao
5. Write the SGID/SUID executable file: shawtao@virt: ~# ./write

Root cause && fix suggestions:

1. Missing kill SUID/SGID check in v9fs_file_write_iter(), so considering call
   file_modified() in v9fs_file_write_iter() can solve the issue.
2. Normally, the host filesystem(i.e. ext4) can also help to clear the SUID/
   SGID when using security_model=none/passthrough. However, Qemu needs to run as
   root with this setting, thus the CAP_FSETID of the Qemu process helps the write
   operation pass the host filesystem's check and keep the SUID/SGID. (So the 9p
   server in Qemu can drop CAP_FSETID on every write like what
   virtfs-proxy-helper does)

Whom to Acknowledge for Reporting This Issue:
Name: Jietao Xiao email: shawtao1125@gmail.com
Name: Wenbo Shen email: shenwenbo@zju.edu.cn
Name: Jinku Li email: jkli@xidian.edu.cn
Name: Nanzi Yang email: nzyang@stu.xidian.edu.cn

Best regards
Jietao Xiao

#!/bin/bash

dd if=/dev/zero of=suid_file bs=1024 count=1

chmod u+s,g+s,o+w,ugo+x suid_file

[carnil]

This appears to be CVE-2023-1386

[rfrohl]

Is there any reasoning why this was closed as NOTABUG by RedHat that someone can share?

[ericvh]

FWIW - I have a private branch where I've been looking to implement some additional protections in the fs/9p client. At the very least we should be doing what NFS is doing and clearing any cached meta-data when we detect the situation so that we pull the revised meta-data from the server. However, this still depends on the server (or the underlying served file system) doing the right thing. There's a more aggressive mode where we try to enforce the semantics from the client by sending extra messages, but assuming downstream is doing the right things that would duplicate effort and add latency and could lead to incorrect behavior - so I've held off on this. I'll try to finish up the meta-data clearing patches this week. If I had to guess why this was closed as NOTABUG it may be because the view is that there are mount/server configurations which prevent this problem (ie. don't let your 9p client write anything as root if you don't trust the guest/client and instead use mapped ids, etc.).

…

On Tue, Aug 8, 2023 at 4:56 AM Robert ***@***.***> wrote: Is there any reasoning why this was closed as NOTABUG by RedHat that someone can share? — Reply to this email directly, view it on GitHub <#29 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAQ7WFJ5AZWROHCCYVFSSDXUIEOFANCNFSM6AAAAAAVR2V4L4> . You are receiving this because you authored the thread.Message ID: ***@***.***>