Merge pull request #5168 from mheon/do_not_overwrite_volumes

Do not copy up when volume is not empty

libpod/container_internal.go

@@ -1383,18 +1383,34 @@ 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())
+
+		// Set NeedsCopyUp to false immediately, so we don't try this
+		// again when there are already files copied.
+		vol.state.NeedsCopyUp = false
+		if err := vol.save(); err != nil {
+			return nil, err
+		}
+
+		// 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())
 		}
-		if err := c.copyWithTarFromImage(srcDir, vol.MountPoint()); err != nil && !os.IsNotExist(err) {
+		if err := c.copyWithTarFromImage(srcDir, volMount); err != nil && !os.IsNotExist(err) {
 			return nil, errors.Wrapf(err, "error copying content from container %s into volume %s", c.ID(), vol.Name())
 		}
-
-		vol.state.NeedsCopyUp = false
-		if err := vol.save(); err != nil {
-			return nil, err
-		}
 	}
 	return vol, nil
 }

test/e2e/run_volume_test.go

@@ -397,4 +397,28 @@ var _ = Describe("Podman run with volumes", func() {
 		volMount.WaitWithDefaultTimeout()
 		Expect(volMount.ExitCode()).To(Not(Equal(0)))
 	})
+
+	It("Podman fix for CVE-2020-1726", func() {
+		volName := "testVol"
+		volCreate := podmanTest.Podman([]string{"volume", "create", volName})
+		volCreate.WaitWithDefaultTimeout()
+		Expect(volCreate.ExitCode()).To(Equal(0))
+
+		volPath := podmanTest.Podman([]string{"volume", "inspect", "--format", "{{.Mountpoint}}", volName})
+		volPath.WaitWithDefaultTimeout()
+		Expect(volPath.ExitCode()).To(Equal(0))
+		path := volPath.OutputToString()
+
+		fileName := "thisIsATestFile"
+		file, err := os.Create(filepath.Join(path, fileName))
+		Expect(err).To(BeNil())
+		defer file.Close()
+
+		runLs := podmanTest.Podman([]string{"run", "-t", "-i", "--rm", "-v", fmt.Sprintf("%v:/etc/ssl", volName), ALPINE, "ls", "-1", "/etc/ssl"})
+		runLs.WaitWithDefaultTimeout()
+		Expect(runLs.ExitCode()).To(Equal(0))
+		outputArr := runLs.OutputToStringArray()
+		Expect(len(outputArr)).To(Equal(1))
+		Expect(strings.Contains(outputArr[0], fileName)).To(BeTrue())
+	})
 })