From b679343237534ba89396fe3eebf38caa77c04d6f Mon Sep 17 00:00:00 2001 From: Song Zhang Date: Thu, 12 Sep 2024 14:15:18 +0800 Subject: [PATCH] runc: fix CVE-2024-45310 Signed-off-by: Song Zhang --- git-commit | 2 +- ...ope-MkdirAll-to-stay-inside-the-root.patch | 361 ++++++++++++++++++ runc.spec | 8 +- series.conf | 1 + 4 files changed, 370 insertions(+), 2 deletions(-) create mode 100644 patch/0159-rootfs-try-to-scope-MkdirAll-to-stay-inside-the-root.patch diff --git a/git-commit b/git-commit index 9d88b24..5516d21 100644 --- a/git-commit +++ b/git-commit @@ -1 +1 @@ -d36503f0bdc96da64702b73054e26c8e6780e8de +71a579d43b489206a17ee296bd001492a7b2ba30 diff --git a/patch/0159-rootfs-try-to-scope-MkdirAll-to-stay-inside-the-root.patch b/patch/0159-rootfs-try-to-scope-MkdirAll-to-stay-inside-the-root.patch new file mode 100644 index 0000000..96204cb --- /dev/null +++ b/patch/0159-rootfs-try-to-scope-MkdirAll-to-stay-inside-the-root.patch @@ -0,0 +1,361 @@ +From 346320978d1b5e7df9d19f51072730b6b332ba98 Mon Sep 17 00:00:00 2001 +From: Aleksa Sarai +Date: Tue, 2 Jul 2024 20:58:43 +1000 +Subject: [PATCH] rootfs: try to scope MkdirAll to stay inside the rootfs + +While we use SecureJoin to try to make all of our target paths inside +the container safe, SecureJoin is not safe against an attacker than can +change the path after we "resolve" it. + +os.MkdirAll can inadvertently follow symlinks and thus an attacker could +end up tricking runc into creating empty directories on the host (note +that the container doesn't get access to these directories, and the host +just sees empty directories). However, this could potentially cause DoS +issues by (for instance) creating a directory in a conf.d directory for +a daemon that doesn't handle subdirectories properly. + +In addition, the handling for creating file bind-mounts did a plain +open(O_CREAT) on the SecureJoin'd path, which is even more obviously +unsafe (luckily we didn't use O_TRUNC, or this bug could've allowed an +attacker to cause data loss...). Regardless of the symlink issue, +opening an untrusted file could result in a DoS if the file is a hung +tty or some other "nasty" file. We can use mknodat to safely create a +regular file without opening anything anyway (O_CREAT|O_EXCL would also +work but it makes the logic a bit more complicated, and we don't want to +open the file for any particular reason anyway). + +libpathrs[1] is the long-term solution for these kinds of problems, but +for now we can patch this particular issue by creating a more restricted +MkdirAll that refuses to resolve symlinks and does the creation using +file descriptors. This is loosely based on a more secure version that +filepath-securejoin now has[2] and will be added to libpathrs soon[3]. + +[1]: https://github.com/openSUSE/libpathrs +[2]: https://github.com/cyphar/filepath-securejoin/releases/tag/v0.3.0 +[3]: https://github.com/openSUSE/libpathrs/issues/10 + +Fixes: CVE-2024-45310 +Signed-off-by: Aleksa Sarai +--- + libcontainer/rootfs_linux.go | 20 ++--- + libcontainer/system/linux.go | 44 +++++++++++ + libcontainer/utils/utils_unix.go | 128 ++++++++++++++++++++++++++++++- + 3 files changed, 182 insertions(+), 10 deletions(-) + +diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go +index d43867b..fbcd7e1 100644 +--- a/libcontainer/rootfs_linux.go ++++ b/libcontainer/rootfs_linux.go +@@ -1,3 +1,4 @@ ++//go:build linux + // +build linux + + package libcontainer +@@ -18,6 +19,7 @@ import ( + "github.com/docker/docker/pkg/mount" + "github.com/docker/docker/pkg/symlink" + "github.com/mrunalp/fileutils" ++ "github.com/opencontainers/runc.bak/libcontainer/utils" + "github.com/opencontainers/runc/libcontainer/cgroups" + "github.com/opencontainers/runc/libcontainer/configs" + "github.com/opencontainers/runc/libcontainer/system" +@@ -192,13 +194,13 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error { + if strings.HasPrefix(m.Destination, "/proc/sys/") { + return nil + } +- if err := os.MkdirAll(dest, 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, dest, 0755); err != nil { + return err + } + // Selinux kernels do not support labeling of /proc or /sys + return mountPropagate(m, rootfs, "") + case "mqueue": +- if err := os.MkdirAll(dest, 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, dest, 0755); err != nil { + return err + } + if err := mountPropagate(m, rootfs, mountLabel); err != nil { +@@ -213,7 +215,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error { + copyUp := m.Extensions&configs.EXT_COPYUP == configs.EXT_COPYUP + tmpDir := "" + if stat, err := os.Stat(dest); err != nil { +- if err := os.MkdirAll(dest, 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, dest, 0755); err != nil { + return err + } + } else { +@@ -270,7 +272,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error { + } + // update the mount with the correct dest after symlinks are resolved. + m.Destination = dest +- if err := createIfNotExists(dest, stat.IsDir()); err != nil { ++ if err := createIfNotExists(rootfs, dest, stat.IsDir()); err != nil { + return err + } + if err := mountPropagate(m, rootfs, mountLabel); err != nil { +@@ -358,7 +360,7 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string) error { + } + // update the mount with the correct dest after symlinks are resolved. + m.Destination = dest +- if err := os.MkdirAll(dest, 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, dest, 0755); err != nil { + return err + } + return mountPropagate(m, rootfs, mountLabel) +@@ -524,7 +526,7 @@ func createDeviceNode(rootfs string, node *configs.Device, bind bool) error { + if err != nil { + return err + } +- if err := os.MkdirAll(filepath.Dir(dest), 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, filepath.Dir(dest), 0755); err != nil { + return err + } + +@@ -733,13 +735,13 @@ func msMoveRoot(rootfs string) error { + } + + // createIfNotExists creates a file or a directory only if it does not already exist. +-func createIfNotExists(path string, isDir bool) error { ++func createIfNotExists(rootfs, path string, isDir bool) error { + if _, err := os.Stat(path); err != nil { + if os.IsNotExist(err) { + if isDir { +- return os.MkdirAll(path, 0755) ++ return utils.MkdirAllInRoot(rootfs, path, 0755) + } +- if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { ++ if err := utils.MkdirAllInRoot(rootfs, filepath.Dir(path), 0755); err != nil { + return err + } + f, err := os.OpenFile(path, os.O_CREATE, 0755) +diff --git a/libcontainer/system/linux.go b/libcontainer/system/linux.go +index 1afc52b..9aaaada 100644 +--- a/libcontainer/system/linux.go ++++ b/libcontainer/system/linux.go +@@ -1,3 +1,4 @@ ++//go:build linux + // +build linux + + package system +@@ -7,8 +8,12 @@ import ( + "fmt" + "os" + "os/exec" ++ "runtime" ++ "strings" + "syscall" + "unsafe" ++ ++ "golang.org/x/sys/unix" + ) + + // If arg2 is nonzero, set the "child subreaper" attribute of the +@@ -134,6 +139,45 @@ func SetSubreaper(i int) error { + return Prctl(PR_SET_CHILD_SUBREAPER, uintptr(i), 0, 0, 0) + } + ++func prepareAt(dir *os.File, path string) (int, string) { ++ if dir == nil { ++ return unix.AT_FDCWD, path ++ } ++ ++ // Rather than just filepath.Join-ing path here, do it manually so the ++ // error and handle correctly indicate cases like path=".." as being ++ // relative to the correct directory. The handle.Name() might end up being ++ // wrong but because this is (currently) only used in MkdirAllInRoot, that ++ // isn't a problem. // isn't a problem. ++ dirName := dir.Name() ++ if !strings.HasSuffix(dirName, "/") { ++ dirName += "/" ++ } ++ fullPath := dirName + path ++ ++ return int(dir.Fd()), fullPath ++} ++ ++func Openat(dir *os.File, path string, flags int, mode uint32) (*os.File, error) { ++ dirFd, fullPath := prepareAt(dir, path) ++ fd, err := unix.Openat(dirFd, path, flags, mode) ++ if err != nil { ++ return nil, &os.PathError{Op: "openat", Path: fullPath, Err: err} ++ } ++ runtime.KeepAlive(dir) ++ return os.NewFile(uintptr(fd), fullPath), nil ++} ++ ++func Mkdirat(dir *os.File, path string, mode uint32) error { ++ dirFd, fullPath := prepareAt(dir, path) ++ err := unix.Mkdirat(dirFd, path, mode) ++ if err != nil { ++ err = &os.PathError{Op: "mkdirat", Path: fullPath, Err: err} ++ } ++ runtime.KeepAlive(dir) ++ return err ++} ++ + func Prctl(option int, arg2, arg3, arg4, arg5 uintptr) (err error) { + _, _, e1 := syscall.Syscall6(syscall.SYS_PRCTL, uintptr(option), arg2, arg3, arg4, arg5, 0) + if e1 != 0 { +diff --git a/libcontainer/utils/utils_unix.go b/libcontainer/utils/utils_unix.go +index cfacfc2..a19f17c 100644 +--- a/libcontainer/utils/utils_unix.go ++++ b/libcontainer/utils/utils_unix.go +@@ -5,17 +5,20 @@ + package utils + + import ( ++ "errors" + "fmt" + "math" + "os" + "path/filepath" + "runtime" + "strconv" ++ "strings" + "sync" + _ "unsafe" // for go:linkname + +- securejoin "github.com/cyphar/filepath-securejoin" + "github.com/Sirupsen/logrus" ++ securejoin "github.com/cyphar/filepath-securejoin" ++ "github.com/opencontainers/runc/libcontainer/system" + "golang.org/x/sys/unix" + ) + +@@ -157,6 +160,20 @@ func NewSockPair(name string) (parent, child *os.File, err error) { + return os.NewFile(uintptr(fds[1]), name+"-p"), os.NewFile(uintptr(fds[0]), name+"-c"), nil + } + ++// IsLexicallyInRoot is shorthand for strings.HasPrefix(path+"/", root+"/"), ++// but properly handling the case where path or root are "/". ++// ++// NOTE: The return value only make sense if the path doesn't contain "..". ++func IsLexicallyInRoot(root, path string) bool { ++ if root != "/" { ++ root += "/" ++ } ++ if path != "/" { ++ path += "/" ++ } ++ return strings.HasPrefix(path, root) ++} ++ + // WithProcfd runs the passed closure with a procfd path (/proc/self/fd/...) + // corresponding to the unsafePath resolved within the root. Before passing the + // fd, this path is verified to have been inside the root -- so operating on it +@@ -262,3 +279,112 @@ func ProcThreadSelf(subpath string) (string, ProcThreadSelfCloser) { + func ProcThreadSelfFd(fd uintptr) (string, ProcThreadSelfCloser) { + return ProcThreadSelf("fd/" + strconv.FormatUint(uint64(fd), 10)) + } ++ ++// MkdirAllInRootOpen attempts to make ++// ++// path, _ := securejoin.SecureJoin(root, unsafePath) ++// os.MkdirAll(path, mode) ++// os.Open(path) ++// ++// safer against attacks where components in the path are changed between ++// SecureJoin returning and MkdirAll (or Open) being called. In particular, we ++// try to detect any symlink components in the path while we are doing the ++// MkdirAll. ++// ++// NOTE: Unlike os.MkdirAll, mode is not Go's os.FileMode, it is the unix mode ++// (the suid/sgid/sticky bits are not the same as for os.FileMode). ++// ++// NOTE: If unsafePath is a subpath of root, we assume that you have already ++// called SecureJoin and so we use the provided path verbatim without resolving ++// any symlinks (this is done in a way that avoids symlink-exchange races). ++// This means that the path also must not contain ".." elements, otherwise an ++// error will occur. ++// ++// This is a somewhat less safe alternative to ++// , but it should ++// detect attempts to trick us into creating directories outside of the root. ++// We should migrate to securejoin.MkdirAll once it is merged. ++func MkdirAllInRootOpen(root, unsafePath string, mode uint32) (_ *os.File, Err error) { ++ // If the path is already "within" the root, use it verbatim. ++ fullPath := unsafePath ++ if !IsLexicallyInRoot(root, unsafePath) { ++ var err error ++ fullPath, err = securejoin.SecureJoin(root, unsafePath) ++ if err != nil { ++ return nil, err ++ } ++ } ++ subPath, err := filepath.Rel(root, fullPath) ++ if err != nil { ++ return nil, err ++ } ++ ++ // Check for any silly mode bits. ++ if mode&^0o7777 != 0 { ++ return nil, fmt.Errorf("tried to include non-mode bits in MkdirAll mode: 0o%.3o", mode) ++ } ++ ++ currentDir, err := os.OpenFile(root, unix.O_DIRECTORY|unix.O_CLOEXEC, 0) ++ if err != nil { ++ return nil, fmt.Errorf("open root handle: %w", err) ++ } ++ defer func() { ++ if Err != nil { ++ currentDir.Close() ++ } ++ }() ++ ++ for _, part := range strings.Split(subPath, string(filepath.Separator)) { ++ switch part { ++ case "", ".": ++ // Skip over no-op components. ++ continue ++ case "..": ++ return nil, fmt.Errorf("possible breakout detected: found %q component in SecureJoin subpath %s", part, subPath) ++ } ++ ++ nextDir, err := system.Openat(currentDir, part, unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) ++ switch { ++ case err == nil: ++ // Update the currentDir. ++ _ = currentDir.Close() ++ currentDir = nextDir ++ ++ case errors.Is(err, unix.ENOTDIR): ++ // This might be a symlink or some other random file. Either way, ++ // error out. ++ return nil, fmt.Errorf("cannot mkdir in %s/%s: %w", currentDir.Name(), part, unix.ENOTDIR) ++ ++ case errors.Is(err, os.ErrNotExist): ++ // Luckily, mkdirat will not follow trailing symlinks, so this is ++ // safe to do as-is. ++ if err := system.Mkdirat(currentDir, part, mode); err != nil { ++ return nil, err ++ } ++ // Open the new directory. There is a race here where an attacker ++ // could swap the directory with a different directory, but ++ // MkdirAll's fuzzy semantics mean we don't care about that. ++ nextDir, err := system.Openat(currentDir, part, unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) ++ if err != nil { ++ return nil, fmt.Errorf("open newly created directory: %w", err) ++ } ++ // Update the currentDir. ++ _ = currentDir.Close() ++ currentDir = nextDir ++ ++ default: ++ return nil, err ++ } ++ } ++ return currentDir, nil ++} ++ ++// MkdirAllInRoot is a wrapper around MkdirAllInRootOpen which closes the ++// returned handle, for callers that don't need to use it. ++func MkdirAllInRoot(root, unsafePath string, mode uint32) error { ++ f, err := MkdirAllInRootOpen(root, unsafePath, mode) ++ if err == nil { ++ _ = f.Close() ++ } ++ return err ++} +-- +2.33.0 + diff --git a/runc.spec b/runc.spec index bc0078e..ccc9957 100644 --- a/runc.spec +++ b/runc.spec @@ -4,7 +4,7 @@ Name: docker-runc Version: 1.0.0.rc3 -Release: 322 +Release: 323 Summary: runc is a CLI tool for spawning and running containers according to the OCI specification. License: ASL 2.0 @@ -57,6 +57,12 @@ install -p -m 755 runc $RPM_BUILD_ROOT/%{_bindir}/runc %{_bindir}/runc %changelog +* Thu Sep 12 2024 Song Zhang - 1.0.0.rc3-323 +- Type:CVE +- CVE:CVE-2024-45310 +- SUG:NA +- DESC:fix CVE-2024-45310 + * Mon Mar 04 2024 zhongjiawei - 1.0.0.rc3-322 - Type:bugfix - CVE:NA diff --git a/series.conf b/series.conf index 6e4541f..875d8bf 100644 --- a/series.conf +++ b/series.conf @@ -158,4 +158,5 @@ patch/0155-runc-fix-CVE-2024-21626.patch patch/0156-runc-check-cmd-exist.patch patch/0157-runc-Fix-File-to-Close.patch patch/0158-runc-Fix-tmpfs-mode-opts-when-dir-already-exis.patch +patch/0159-rootfs-try-to-scope-MkdirAll-to-stay-inside-the-root.patch #end -- Gitee