From 9be1dbf4ac67d9840a043ebd2df5c68f36705d1d Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 15 May 2025 17:32:01 +1000 Subject: [PATCH] console: avoid trivial symlink attacks for /dev/console An attacker could make /dev/console a symlink. This presents two possible issues: 1. os.Create will happily truncate targets, which could have resulted in a worse version of CVE-2024-4531. Luckily, this all happens after pivot_root(2) so the scope of that particular attack is fairly limited (you are unlikely to be able to easily access host rootfs files -- though it might be possible to take advantage of leaks such as in CVE-2024-21626). However, O_CREAT|O_NOFOLLOW is what we should be doing for all file creations. 2. Because we passed /dev/console as the only mount path (as opposed to using a /proc/self/fd/$n path), an attacker could swap the symlink to point to any other path and thus cause us to mount over some other path. This is not as big of a problem because all the mounts are in the container namespace after pivot_root(2), and users usually can create arbitrary mount targets inside the container. These issues don't seem particularly exploitable, but they deserve to be hardened regardless. Signed-off-by: Aleksa Sarai --- libcontainer/console_linux.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/libcontainer/console_linux.go b/libcontainer/console_linux.go index 418c27f25..461ae8e98 100644 --- a/libcontainer/console_linux.go +++ b/libcontainer/console_linux.go @@ -12,6 +12,7 @@ import ( "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/internal/pathrs" "github.com/opencontainers/runc/internal/sys" + "github.com/opencontainers/runc/libcontainer/utils" ) func isPtyNoIoctlError(err error) bool { @@ -87,22 +88,20 @@ func safeAllocPty() (pty console.Console, peer *os.File, Err error) { // mountConsole bind-mounts the provided pty on top of /dev/console so programs // that operate on /dev/console operate on the correct container pty. func mountConsole(peerPty *os.File) error { - f, err := os.Create("/dev/console") - if err != nil && !os.IsExist(err) { - return err - } - if f != nil { - // Ensure permission bits (can be different because of umask). - if err := f.Chmod(0o666); err != nil { - return err - } - f.Close() + console, err := os.OpenFile("/dev/console", unix.O_NOFOLLOW|unix.O_CREAT|unix.O_CLOEXEC, 0o666) + if err != nil { + return fmt.Errorf("create /dev/console mount target: %w", err) } + defer console.Close() + + dstFd, closer := utils.ProcThreadSelfFd(console.Fd()) + defer closer() + mntSrc := &mountSource{ Type: mountSourcePlain, file: peerPty, } - return mountViaFds(peerPty.Name(), mntSrc, "/dev/console", "", "bind", unix.MS_BIND, "") + return mountViaFds(peerPty.Name(), mntSrc, "/dev/console", dstFd, "bind", unix.MS_BIND, "") } // dupStdio replaces stdio with the given peerPty.