From 8476df83b534a2522b878c0507b3491def48db9f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 6 Mar 2025 08:19:45 -0800 Subject: [PATCH] libct: add/use isDevNull, verifyDevNull The /dev/null in a container should not be trusted, because when /dev is a bind mount, /dev/null is not created by runc itself. 1. Add isDevNull which checks the fd minor/major and device type, and verifyDevNull which does the stat and the check. 2. Rewrite maskPath to open and check /dev/null, and use its fd to perform mounts. Move the loop over the MaskPaths into the function, and rename it to maskPaths. 3. reOpenDevNull: use verifyDevNull and isDevNull. 4. fixStdioPermissions: use isDevNull instead of stat. Fixes: GHSA-9493-h29p-rfm2 CVE-2025-31133 Co-authored-by: Rodrigo Campos Signed-off-by: Kir Kolyshkin Signed-off-by: Aleksa Sarai --- libcontainer/init_linux.go | 11 +++--- libcontainer/rootfs_linux.go | 55 ++++++++++++++++++++++------- libcontainer/standard_init_linux.go | 7 ++-- 3 files changed, 50 insertions(+), 23 deletions(-) diff --git a/libcontainer/init_linux.go b/libcontainer/init_linux.go index e4daf2c8f..95a4d7f78 100644 --- a/libcontainer/init_linux.go +++ b/libcontainer/init_linux.go @@ -505,19 +505,16 @@ func setupUser(config *initConfig) error { // The ownership needs to match because it is created outside of the container and needs to be // localized. func fixStdioPermissions(uid int) error { - var null unix.Stat_t - if err := unix.Stat("/dev/null", &null); err != nil { - return &os.PathError{Op: "stat", Path: "/dev/null", Err: err} - } for _, file := range []*os.File{os.Stdin, os.Stdout, os.Stderr} { var s unix.Stat_t if err := unix.Fstat(int(file.Fd()), &s); err != nil { return &os.PathError{Op: "fstat", Path: file.Name(), Err: err} } - // Skip chown if uid is already the one we want or any of the STDIO descriptors - // were redirected to /dev/null. - if int(s.Uid) == uid || s.Rdev == null.Rdev { + // Skip chown if: + // - uid is already the one we want, or + // - fd is opened to /dev/null. + if int(s.Uid) == uid || isDevNull(&s) { continue } diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index dddd709b8..552734f56 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -26,6 +26,7 @@ import ( "github.com/opencontainers/cgroups/fs2" "github.com/opencontainers/runc/internal/linux" "github.com/opencontainers/runc/internal/pathrs" + "github.com/opencontainers/runc/internal/sys" "github.com/opencontainers/runc/libcontainer/configs" "github.com/opencontainers/runc/libcontainer/utils" ) @@ -398,7 +399,7 @@ func mountCgroupV2(m *configs.Mount, c *mountConfig) error { // Mask `/sys/fs/cgroup` to ensure it is read-only, even when `/sys` is mounted // with `rbind,ro` (`runc spec --rootless` produces `rbind,ro` for `/sys`). err = utils.WithProcfd(c.root, m.Destination, func(procfd string) error { - return maskPath(procfd, c.label) + return maskPaths([]string{procfd}, c.label) }) } return err @@ -889,20 +890,20 @@ func setupDevSymlinks(rootfs string) error { // needs to be called after we chroot/pivot into the container's rootfs so that any // symlinks are resolved locally. func reOpenDevNull() error { - var stat, devNullStat unix.Stat_t file, err := os.OpenFile("/dev/null", os.O_RDWR, 0) if err != nil { return err } defer file.Close() - if err := unix.Fstat(int(file.Fd()), &devNullStat); err != nil { - return &os.PathError{Op: "fstat", Path: file.Name(), Err: err} + if err := verifyDevNull(file); err != nil { + return fmt.Errorf("can't reopen /dev/null: %w", err) } for fd := range 3 { + var stat unix.Stat_t if err := unix.Fstat(fd, &stat); err != nil { return &os.PathError{Op: "fstat", Path: "fd " + strconv.Itoa(fd), Err: err} } - if stat.Rdev == devNullStat.Rdev { + if isDevNull(&stat) { // Close and re-open the fd. if err := linux.Dup3(int(file.Fd()), fd, 0); err != nil { return err @@ -1251,18 +1252,48 @@ func remountReadonly(m *configs.Mount) error { return fmt.Errorf("unable to mount %s as readonly max retries reached", dest) } -// maskPath masks the top of the specified path inside a container to avoid +func isDevNull(st *unix.Stat_t) bool { + return st.Mode&unix.S_IFMT == unix.S_IFCHR && st.Rdev == unix.Mkdev(1, 3) +} + +func verifyDevNull(f *os.File) error { + return sys.VerifyInode(f, func(st *unix.Stat_t, _ *unix.Statfs_t) error { + if !isDevNull(st) { + return errors.New("container's /dev/null is invalid") + } + return nil + }) +} + +// maskPaths masks the top of the specified paths inside a container to avoid // security issues from processes reading information from non-namespace aware // mounts ( proc/kcore ). // For files, maskPath bind mounts /dev/null over the top of the specified path. // For directories, maskPath mounts read-only tmpfs over the top of the specified path. -func maskPath(path string, mountLabel string) error { - if err := mount("/dev/null", path, "", unix.MS_BIND, ""); err != nil && !errors.Is(err, os.ErrNotExist) { - if errors.Is(err, unix.ENOTDIR) { - return mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("", mountLabel)) - } - return err +func maskPaths(paths []string, mountLabel string) error { + devNull, err := os.OpenFile("/dev/null", unix.O_PATH, 0) + if err != nil { + return fmt.Errorf("can't mask paths: %w", err) } + defer devNull.Close() + if err := verifyDevNull(devNull); err != nil { + return fmt.Errorf("can't mask paths: %w", err) + } + devNullSrc := &mountSource{Type: mountSourcePlain, file: devNull} + + for _, path := range paths { + if err := mountViaFds("", devNullSrc, path, "", "", unix.MS_BIND, ""); err != nil && !errors.Is(err, os.ErrNotExist) { + if !errors.Is(err, unix.ENOTDIR) { + return fmt.Errorf("can't mask path %q: %w", path, err) + } + // Destination is a directory: bind mount a ro tmpfs over it. + err := mount("tmpfs", path, "tmpfs", unix.MS_RDONLY, label.FormatMountLabel("", mountLabel)) + if err != nil { + return fmt.Errorf("can't mask dir %q: %w", path, err) + } + } + } + return nil } diff --git a/libcontainer/standard_init_linux.go b/libcontainer/standard_init_linux.go index f8023f391..d3e519593 100644 --- a/libcontainer/standard_init_linux.go +++ b/libcontainer/standard_init_linux.go @@ -142,10 +142,9 @@ func (l *linuxStandardInit) Init() error { return fmt.Errorf("can't make %q read-only: %w", path, err) } } - for _, path := range l.config.Config.MaskPaths { - if err := maskPath(path, l.config.Config.MountLabel); err != nil { - return fmt.Errorf("can't mask path %s: %w", path, err) - } + + if err := maskPaths(l.config.Config.MaskPaths, l.config.Config.MountLabel); err != nil { + return err } pdeath, err := system.GetParentDeathSignal() if err != nil {