From c0627de21de4c19d7dd4c2541949033f95dae0cf Mon Sep 17 00:00:00 2001
From: Giuseppe Scrivano <gscrivan@redhat.com>
Date: Tue, 18 Mar 2025 12:18:12 +0100
Subject: [PATCH] container: replace code with securejoin.OpenInRoot()

when the code was first added, there was no securejoin.OpenInRoot().
Since there is a function already provided by a dependency and already
used in libpod, replace the custom code with securejoin.OpenInRoot().

The new version does not report a symlink that points outside the
root, but it is still resolved relative to the specified mountpoint,
since that is the openat2 semantic.  It does not affect the security
of the function.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
---
 libpod/container_internal_linux.go | 36 ++++++++----------------------
 test/e2e/play_kube_test.go         |  6 ++---
 2 files changed, 11 insertions(+), 31 deletions(-)

diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index 3a3704c2ff..bb1ac393bf 100644
--- a/libpod/container_internal_linux.go
+++ b/libpod/container_internal_linux.go
@@ -741,41 +741,22 @@ func (s *safeMountInfo) Close() {
 // The caller is responsible for closing the file descriptor and unmounting the subpath
 // when it's no longer needed.
 func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountInfo, err error) {
-	joinedPath := filepath.Clean(filepath.Join(mountPoint, subpath))
-	fd, err := unix.Open(joinedPath, unix.O_RDONLY|unix.O_PATH, 0)
+	file, err := securejoin.OpenInRoot(mountPoint, subpath)
 	if err != nil {
 		return nil, err
 	}
-	f := os.NewFile(uintptr(fd), joinedPath)
-	defer func() {
-		if err != nil {
-			f.Close()
-		}
-	}()
-
-	// Once we got the file descriptor, we need to check that the subpath is a valid.  We
-	// refer to the open FD so there won't be other path lookups (and no risk to follow a symlink).
-	fdPath := fmt.Sprintf("/proc/%d/fd/%d", os.Getpid(), f.Fd())
-	p, err := os.Readlink(fdPath)
-	if err != nil {
-		return nil, err
-	}
-	relPath, err := filepath.Rel(mountPoint, p)
-	if err != nil {
-		return nil, err
-	}
-	if relPath == ".." || strings.HasPrefix(relPath, "../") {
-		return nil, fmt.Errorf("subpath %q is outside of the volume %q", subpath, mountPoint)
-	}
 
-	fi, err := os.Stat(fdPath)
+	// we need to always reference the file by its fd, that points inside the mountpoint.
+	fname := fmt.Sprintf("/proc/self/fd/%d", int(file.Fd()))
+
+	fi, err := os.Stat(fname)
 	if err != nil {
 		return nil, err
 	}
 	var npath string
 	switch {
 	case fi.Mode()&fs.ModeSymlink != 0:
-		return nil, fmt.Errorf("file %q is a symlink", joinedPath)
+		return nil, fmt.Errorf("file %q is a symlink", filepath.Join(mountPoint, subpath))
 	case fi.IsDir():
 		npath, err = os.MkdirTemp(c.state.RunDir, "subpath")
 		if err != nil {
@@ -789,11 +770,12 @@ func (c *Container) safeMountSubPath(mountPoint, subpath string) (s *safeMountIn
 		tmp.Close()
 		npath = tmp.Name()
 	}
-	if err := unix.Mount(fdPath, npath, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
+
+	if err := unix.Mount(fname, npath, "", unix.MS_BIND|unix.MS_REC, ""); err != nil {
 		return nil, err
 	}
 	return &safeMountInfo{
-		file:       f,
+		file:       file,
 		mountPoint: npath,
 	}, nil
 }
diff --git a/test/e2e/play_kube_test.go b/test/e2e/play_kube_test.go
index a739618249..078bad5533 100644
--- a/test/e2e/play_kube_test.go
+++ b/test/e2e/play_kube_test.go
@@ -5545,7 +5545,7 @@ spec:
 
 		playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml})
 		playKube.WaitWithDefaultTimeout()
-		Expect(playKube).Should(ExitWithError(125, fmt.Sprintf(`subpath "testing/onlythis" is outside of the volume "%s/root/volumes/testvol/_data`, podmanTest.TempDir)))
+		Expect(playKube).Should(ExitWithError(125, fmt.Sprintf("securejoin.OpenInRoot testing/onlythis: openat2 %s/root/volumes/testvol/_data/testing/onlythis: no such file or directory", podmanTest.TempDir)))
 	})
 
 	It("with unsafe hostPath subpaths", func() {
@@ -5559,9 +5559,7 @@ spec:
 		err = generateKubeYaml("pod", pod, kubeYaml)
 		Expect(err).To(Not(HaveOccurred()))
 
-		playKube := podmanTest.Podman([]string{"kube", "play", kubeYaml})
-		playKube.WaitWithDefaultTimeout()
-		Expect(playKube).Should(ExitWithError(125, fmt.Sprintf(`subpath "testing/symlink" is outside of the volume "%s"`, hostPathLocation)))
+		podmanTest.PodmanExitCleanly("kube", "play", kubeYaml)
 	})
 
 	It("with configMap subpaths", func() {