Virtio-fs escape outside of specified vm directory

[mtjhrc]

In the current form, it is possible to escape out of directories exposed by the libkruns "fs" device.
In fact, there is even a warning in the source code:

libkrun/src/devices/src/virtio/fs/linux/passthrough.rs

Lines 311 to 316 in 9041aaa

	/// A file system that simply "passes through" all requests it receives to the underlying file
	/// system. To keep the implementation simple it servers the contents of its root directory. Users
	/// that wish to serve only a specific directory should set up the environment so that that
	/// directory ends up as the root of the file system process. One way to accomplish this is via a
	/// combination of mount namespaces and the pivot_root system call.
	pub struct PassthroughFs {

But we don't provide any warning to the user here:

libkrun/include/libkrun.h

Lines 61 to 71 in 45563e9

	/**
	* Sets the path to be use as root for the microVM. Not available in libkrun-SEV.
	*
	* Arguments:
	* "ctx_id" - the configuration context ID.
	* "root_path" - a null-terminated string representing the path to be used as root.
	*
	* Returns:
	* Zero on success or a negative error number on failure.
	*/
	int32_t krun_set_root(uint32_t ctx_id, const char *root_path);

And here:

libkrun/include/libkrun.h

Lines 198 to 208 in 9041aaa

	* Adds an independent virtio-fs device pointing to a host's directory with a tag.
	*
	* Arguments:
	* "ctx_id" - the configuration context ID.
	* "c_tag" - tag to identify the filesystem in the guest.
	* "c_path" - full path to the directory in the host to be exposed to the guest.
	*
	* Returns:
	* Zero on success or a negative error number on failure.
	*/
	int32_t krun_add_virtiofs(uint32_t ctx_id,

The main issue stems from the fact that we basically trust the guest kernel that when it gives us a filename it is actually a filename and not a path. In order to exploit this you need to modify the guest kernel, which should be possible within the VM since you have root access. This is because the normal user-space API obviously resolves path for you and doesn't allow you to attempt to create files with invalid names.

I made a patch for the libkrunfw kernel to play around with this from userspace:
https://gist.github.com/mtjhrc/964e009b2a18e7f1b7311df4b7d8696d
This hacky patch allows you to issue weird fs request from userspace with invalid "filenames" containing / and ... This changes % into / and :: into .. in virtio-fs requests.

I tested this on Linux host, but I think this should also be exploitable on macOS (with slightly different behavior).

I found 2 related variants of this bug:

• putting a path with .. and / in filename of virtio-fs commands:

  Run this inside the VM to create a file named HACKED in your /tmp on the host filesystem:
  # touch ::%::%::%::%::%::%tmp%HACKED
  (The kernel patch changes this to ../../../../../../tmp/HACKED)

  The fix for this should be straight forward - refuse to process fs request from the guest with / in filenames in libkrun.

• resolving .. of /
  I feel like this is slightly different than the first issue, because it could make sense for the guest to use .. to refer to parent directories that are still inside the VM directory (but I'm not sure, if this is a valid FUSE operation).

  You can try this with this python script:
  # python3 -c 'import os; os.write(os.open("HACKED", os.O_WRONLY | os.O_CREAT, dir_fd=os.open("::", os.O_PATH)), b"HACKED!");' - this should create a file named HACKED 1 level outside of the root directory of the VM.

  The fix for this should probably be more nuanced and resolve .. of / to just another reference to /, but for other directories, .. can be left to work normally (maybe?). I don't know what is the proper fix is - should .. be disallowed completely or just the path carefully resolved for root directory?

Note that this effects more that just the fuse LOOKUP operation - it also effects: unlink, symlink and anything else that deals with filenames.

[slp]

Great work here, @mtjhrc. While it's true that none of our current workloads are affected (crun uses namespaces, podman-machine runs containers confined with namespaces and muvm shared the entire host's rootfs), I agree this is something we need to get addressed.

My main concern is that we need to make sure the solution doesn't break the filesystem semantics. We need to set up a set of filesystems tests and do a before and after verification to ensure we don't introduce any regressions. We should perhaps even add those tests to the CI. We can talk about this next week.