diff options
-rw-r--r-- | libpod/container_internal.go | 28 | ||||
-rw-r--r-- | pkg/api/handlers/libpod/images.go | 2 | ||||
-rw-r--r-- | test/e2e/run_volume_test.go | 24 |
3 files changed, 47 insertions, 7 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 78ec09f29..216bbe669 100644 --- a/libpod/container_internal.go +++ b/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 } diff --git a/pkg/api/handlers/libpod/images.go b/pkg/api/handlers/libpod/images.go index f6459f1eb..bcbe4977e 100644 --- a/pkg/api/handlers/libpod/images.go +++ b/pkg/api/handlers/libpod/images.go @@ -304,7 +304,7 @@ func ImagesPull(w http.ResponseWriter, r *http.Request) { return } else if err != nil { origErr := err - imageRef, err = alltransports.ParseImageName(fmt.Sprintf("%s:%s", docker.Transport.Name(), query.Reference)) + imageRef, err = alltransports.ParseImageName(fmt.Sprintf("%s://%s", docker.Transport.Name(), query.Reference)) if err != nil { utils.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest, errors.Wrapf(origErr, "reference %q must be a docker reference", query.Reference)) diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 0c2389e40..46c27dc2e 100644 --- a/test/e2e/run_volume_test.go +++ b/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()) + }) }) |