1
0
mirror of https://github.com/opencontainers/runc.git synced 2026-02-05 18:45:28 +01:00

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 <rodrigoca@microsoft.com>
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
This commit is contained in:
Kir Kolyshkin
2025-03-06 08:19:45 -08:00
committed by Aleksa Sarai
parent ff94f9991b
commit 8476df83b5
3 changed files with 50 additions and 23 deletions

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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 {