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:
committed by
Aleksa Sarai
parent
ff94f9991b
commit
8476df83b5
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user