mirror of
https://github.com/opencontainers/runc.git
synced 2025-12-24 11:50:58 +08:00
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 <cyphar@cyphar.com>
This commit is contained in:
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user