From c140ecdc9b416ab4efd4d21d14acd63b6adbdd42 Mon Sep 17 00:00:00 2001

From: Matthew Heon <matthew.heon@pm.me>

Date: Mon, 10 Feb 2020 13:37:38 -0500

Subject: [PATCH] Do not copy up when volume is not empty

When Docker performs a copy up, it first verifies that the volume

being copied into is empty; thus, for volumes that have been

modified elsewhere (e.g. manually copying into then), the copy up

will not be performed at all. Duplicate this behavior in Podman

by checking if the volume is empty before copying.

Furthermore, move setting copyup to false further up. This will

prevent a potential race where copy up could happen more than

once if Podman was killed after some files had been copied but

before the DB was updated.

This resolves CVE-2020-1726.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

---

 libpod/container_internal.go | 28 ++++++++++++++++++++++------

 test/e2e/run_volume_test.go  | 24 ++++++++++++++++++++++++

 2 files changed, 46 insertions(+), 6 deletions(-)

diff -up ./libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/libpod/container_internal.go.CVE-2020-1726 ./libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/libpod/container_internal.go

--- libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/libpod/container_internal.go.CVE-2020-1726	2020-02-13 22:37:15.002058706 +0100

+++ libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/libpod/container_internal.go	2020-02-13 22:37:15.005058742 +0100

@@ -1358,18 +1358,34 @@ func (c *Container) mountNamedVolume(v *

 	}

 	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

 }

diff -up ./libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/test/e2e/run_volume_test.go.CVE-2020-1726 ./libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/test/e2e/run_volume_test.go

--- libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/test/e2e/run_volume_test.go.CVE-2020-1726	2020-02-13 22:37:15.030059039 +0100

+++ libpod-5cc92849f7fc9dd734ca2fd8f3ae8830b9a7eb26/test/e2e/run_volume_test.go	2020-02-13 22:37:15.033059075 +0100

@@ -375,4 +375,28 @@ var _ = Describe("Podman run with volume

 		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())

+	})

 })