[CVE-2019-19921]: Volume mount race condition with shared mounts

[leoluk]

Disclosed in #2190.

Here's the original report to security@opencontainers.org:

Hi all,

an attacker who controls the container image for two containers that share a volume can race volume mounts during container initialization, by adding a symlink to the rootfs that points to a directory on the volume. The second container won't be able to see the actual mount, but it can race it by modifying the mount point on the volume.

This can be exploited for a full container breakout by racing readonly/mask mounts, allowing writes to dangerous paths like /proc/sys/kernel/core_pattern.

Example:

• The rootfs of container A has a symlink /proc -> /evil/level1
• Container A specifies a named volume mounted to /evil
• Container B, started before container A, shares this named volume and repeatedly swaps /evil/level1 and /evil/level1~
• Container A mounts procfs to /evil/level1~/level2, but when it remounts /proc/sys, it does so at /evil/level1/level2/sys.

This can reliably be reproduced using runc and podman on Fedora 30 (takes about 0-5s to win the race for me): https://gist.github.com/leoluk/82965ad9df58247202aa0e1878439092

SELinux would ordinarily prevent the exploit by disallowing container_t from writing usermodehelper_t, but it can be disabled by symlinking /proc/self/task/1/attr/exec to something benign like /proc/self/sched (bypassing the procfs check). AppArmor can be disabled similarly.

Docker specifies the mounts in a different order and mounts procfs after it mounts the volumes, mounting over the /proc symlink, which appears to prevent at least the /proc approach. I haven't tested other runc usage scenarios, for instance, k8s+cri-o might be vulnerable as well.

Fabian of Cure53 (in CC) created a minimal PoC that uses runc directly: https://gist.github.com/LiveOverflow/c937820b688922eb127fb760ce06dab9

There are other container init steps after the volume mount that can be raced, obvious ones being utils.CloseExecFrom and the AppArmor/SELinux attrs but there might be others, especially in mountToRootfs (like tricking remount into mounting the rootfs as rshared if there's another volume that specifies the flag, but I haven't tried that).

This is similar to the vulnerability I reported that Adam Iwaniuk disclosed during their Dragon Sector CTF (#2128) and a similar crun one (containers/crun#111).

The fix for the mounts is probably what Aleksa outlined here, using /proc/self/fd to resolve the path: containers/crun#111 (comment)

[cyphar]

My proposed ("stop the bleeding") patch was something like the following:

commit 81a9af6677b1f87e70b87e9a655cb4f4d06a0503 (HEAD -> fix-double-volume-attack)
Author: Aleksa Sarai <asarai@suse.de>
Date:   Sat Dec 21 23:40:17 2019 +1100

    rootfs: do not permit /proc mounts to non-directories
    
    mount(2) will blindly follow symlinks, which is a problem because it
    allows a malicious container to trick runc into mounting /proc to an
    entirely different location (and thus within the attacker's control for
    a rename-exchange attack).
    
    This is just a hotfix, and the more complete fix would be finish
    libpathrs and port runc to it (to avoid these types of attacks entirely,
    and defend against a variety of other /proc-related attacks).
    
    Fixes: CVE-YYYY-XXXX
    Signed-off-by: Aleksa Sarai <asarai@suse.de>

diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go
index 291021440a1a..6e896bc4fdaa 100644
--- a/libcontainer/rootfs_linux.go
+++ b/libcontainer/rootfs_linux.go
@@ -297,17 +297,49 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string, enableCgroupns b
                dest = filepath.Join(rootfs, dest)
        }
 
+       // For "special" filesystems, we have to be quite careful about mounting --
+       // we must make sure that the destination is what we expect. This is done
+       // by opening the destination as an O_PATH descriptor, and using the
+       // /proc/self/fd/... as the mount target. Unfortunately this is actually
+       // possible to bypass with a little bit of thought, but the complete
+       // solution for this will be to port runc to libpathrs.
        switch m.Device {
-       case "proc", "sysfs":
+       case "proc", "sysfs", "mqueue":
+               // NOTE: If the container controls any part of dest, this is unsafe.
                if err := os.MkdirAll(dest, 0755); err != nil {
                        return err
                }
+               destFd, err := unix.Open(dest, unix.O_PATH|unix.O_CLOEXEC, 0)
+               if err != nil {
+                       return err
+               }
+               defer unix.Close(destFd)
+
+               // Check that the path is exactly what we expect.
+               // NOTE: If the path contains an attacker-controlled bind-mount, this
+               //       check won't do anything. In addition, if procfs is fraudulent,
+               //       it will also be useless. As above, the solution is to switch
+               //       to libpathrs.
+               destFdPath := fmt.Sprintf("/proc/self/fd/%d", destFd)
+               destUnsafePath, err := os.Readlink(destFdPath)
+               if err != nil {
+                       return err
+               }
+               if destUnsafePath != dest {
+                       return fmt.Errorf("detected possible breakout: trying to mount '%s' on '%s' was actually targeted to '%s'", m.Device, dest, destUnsafePath)
+               }
+
+               // Okay, now we can use destFdPath.
+               dest = destFdPath
+               m.Destination = destFdPath
+       }
+
+       // Now actually do the mount.
+       switch m.Device {
+       case "proc", "sysfs":
                // Selinux kernels do not support labeling of /proc or /sys
                return mountPropagate(m, rootfs, "")
        case "mqueue":
-               if err := os.MkdirAll(dest, 0755); err != nil {
-                       return err
-               }
                if err := mountPropagate(m, rootfs, mountLabel); err != nil {
                        // older kernels do not support labeling of /dev/mqueue
                        if err := mountPropagate(m, rootfs, ""); err != nil {

Unfortunately this is not sufficient if / is shared with another container, because then you can do the same trick (but this time on / directly). It also needs some more work to work around the fact that there are m.Destination-based checks elsewhere in rootfs_linux.go.

[leoluk]

Your patch does stop the bleeding, though - most runc use cases do not share the rootfs. Mounting a volume on / breaks all kinds of things. Haven't managed to do anything useful using either cri-o or podman.

[cyphar]

Alright, I'll prepare a PR. Thanks @leoluk -- and sorry for the response time issues (as well as how the disclosure happened).

[liggitt]

any ETA on the workaround to unblock rc10?

[cyphar]

I've been off the face of the earth for the past 2ish weeks. I will prepare a PR tomorrow.

[cyphar]

#2207 contains a very simplified version of the above patch (the patch I posted above doesn't work because rootfs_linux.go has a very fun relationship with pathnames that I don't have time to debug right now).