rootfs: prohibit symlinks that conflicts with readonlyPaths, maskedPaths

"/proc" and "/sys" in the container rootfs can no longer be symlinks,
as they could be tricked to obtain the host root.

Fix issue 3751 ("CVE-2019-19921 re-introduction/regression")
Fix CVE-2023-27561

The CVE has been already public, so I'm just going to open this PR publicly.

Co-authored-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Akihiro Suda <[email protected]>

libcontainer/rootfs_linux.go

@@ -52,6 +52,10 @@ func needsSetupDev(config *configs.Config) bool {
 // finalizeRootfs after this function to finish setting up the rootfs.
 func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig, mountFds []int) (err error) {
 	config := iConfig.Config
+	if err := validateRoot(config); err != nil {
+		return fmt.Errorf("error validating rootfs: %w", err)
+	}
+
 	if err := prepareRoot(config); err != nil {
 		return fmt.Errorf("error preparing rootfs: %w", err)
 	}
@@ -1115,3 +1119,37 @@ func setRecAttr(m *configs.Mount, rootfs string) error {
 		return unix.MountSetattr(-1, procfd, unix.AT_RECURSIVE, m.RecAttr)
 	})
 }
+
+// validateRoot detects symlinks in config.Rootfs that may break config.ReadonlyPaths
+// and config.MaskPaths.
+//
+// For preventing CVE-2023-27561 (CVE-2019-19921 re-introduction/regression).
+// https://github.com/opencontainers/runc/issues/3751
+func validateRoot(config *configs.Config) error {
+	mustNotBeSymlink := make(map[string]struct{})
+	for _, f := range append(config.ReadonlyPaths, config.MaskPaths...) {
+		if !filepath.IsAbs(f) || f != filepath.Clean(f) {
+			return fmt.Errorf("bad path: %q", f)
+		}
+		for f != "/" {
+			mustNotBeSymlink[f] = struct{}{}
+			f = filepath.Dir(f)
+		}
+	}
+	// mustNotBeSymlink typically contains {"/sys", "/proc"} in addition to readonlyPaths and maskedPaths.
+	rootFD, err := unix.Open(config.Rootfs, unix.O_DIRECTORY|unix.O_RDONLY, 0o600)
+	if err != nil {
+		return &os.PathError{Op: "open", Path: config.Rootfs, Err: err}
+	}
+	defer unix.Close(rootFD)
+	for f := range mustNotBeSymlink {
+		// fstatat(2): "If pathname is absolute, then dirfd is ignored."
+		fRel := "./" + f
+		var st unix.Stat_t
+		err := unix.Fstatat(rootFD, fRel, &st, unix.AT_SYMLINK_NOFOLLOW)
+		if err == nil && st.Mode&unix.S_IFMT == unix.S_IFLNK {
+			return fmt.Errorf("must not be a symlink (conflicts with readonlyPaths and/or maskedPaths): %q", f)
+		}
+	}
+	return nil
+}

tests/integration/mask.bats

@@ -6,11 +6,12 @@ function setup() {
 	setup_busybox
 
 	# Create fake rootfs.
-	mkdir rootfs/testdir
+	mkdir rootfs/testdir rootfs/testdir2
 	echo "Forbidden information!" >rootfs/testfile
+	echo "Forbidden information! (in a nested dir)" >rootfs/testdir2/file2
 
 	# add extra masked paths
-	update_config '(.. | select(.maskedPaths? != null)) .maskedPaths += ["/testdir", "/testfile"]'
+	update_config '(.. | select(.maskedPaths? != null)) .maskedPaths += ["/testdir", "/testfile", "/testdir2/testfile2"]'
 }
 
 function teardown() {
@@ -56,3 +57,41 @@ function teardown() {
 	[ "$status" -eq 1 ]
 	[[ "${output}" == *"Operation not permitted"* ]]
 }
+
+@test "mask paths [prohibit symlink /proc]" {
+	ln -s /symlink rootfs/proc
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *"must not be a symlink"* ]]
+}
+
+@test "mask paths [prohibit symlink /sys]" {
+	ln -s /symlink rootfs/sys
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *"must not be a symlink"* ]]
+}
+
+@test "mask paths [prohibit symlink /testdir]" {
+	rmdir rootfs/testdir
+	ln -s /symlink rootfs/testdir
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *"must not be a symlink"* ]]
+}
+
+@test "mask paths [prohibit symlink /testfile]" {
+	rm -f rootfs/testfile
+	ln -s /symlink rootfs/testfile
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *"must not be a symlink"* ]]
+}
+
+@test "mask paths [prohibit symlink /testdir2 (parent of /testdir2/testfile2)]" {
+	rm -rf rootfs/testdir2
+	ln -s /symlink rootfs/testdir2
+	runc run -d --console-socket "$CONSOLE_SOCKET" test_busybox
+	[ "$status" -eq 1 ]
+	[[ "${output}" == *"must not be a symlink"* ]]
+}