From 443a2afdb5bf471bca03a6de649e0978b13996ab Mon Sep 17 00:00:00 2001 From: Giuseppe Scrivano Date: Sat, 25 Jun 2022 14:19:04 +0200 Subject: volumes: switch order of checks avoid any I/O operation on the volume if the source directory is empty. This is useful on network file systems (since CAP_DAC_OVERRIDE is not honored) where the root user might not have enough privileges to perform an I/O operation on the NFS mount but the user running inside the container has. [NO NEW TESTS NEEDED] it needs a setup with a network file system Signed-off-by: Giuseppe Scrivano --- libpod/container_internal.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/libpod/container_internal.go b/libpod/container_internal.go index ae61298f3..fe2a19a4e 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1642,19 +1642,6 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) if vol.state.NeedsCopyUp { logrus.Debugf("Copying up contents from container %s to volume %s", c.ID(), vol.Name()) - // If the volume is not empty, we should not copy up. - volMount := vol.mountPoint() - contents, err := ioutil.ReadDir(volMount) - if err != nil { - return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID()) - } - if len(contents) > 0 { - // The volume is not empty. It was likely modified - // outside of Podman. For safety, let's not copy up into - // it. Fixes CVE-2020-1726. - return vol, nil - } - srcDir, err := securejoin.SecureJoin(mountpoint, v.Dest) if err != nil { return nil, errors.Wrapf(err, "error calculating destination path to copy up container %s volume %s", c.ID(), vol.Name()) @@ -1688,6 +1675,19 @@ func (c *Container) mountNamedVolume(v *ContainerNamedVolume, mountpoint string) return vol, nil } + // If the volume is not empty, we should not copy up. + volMount := vol.mountPoint() + contents, err := ioutil.ReadDir(volMount) + if err != nil { + return nil, errors.Wrapf(err, "error listing contents of volume %s mountpoint when copying up from container %s", vol.Name(), c.ID()) + } + if len(contents) > 0 { + // The volume is not empty. It was likely modified + // outside of Podman. For safety, let's not copy up into + // it. Fixes CVE-2020-1726. + return vol, nil + } + // Set NeedsCopyUp to false since we are about to do first copy // Do not copy second time. vol.state.NeedsCopyUp = false -- cgit v1.2.3-54-g00ecf