From 290cfcb00991d606a26d2cee78b7bcc4c78e9dcd Mon Sep 17 00:00:00 2001 From: Michael Wan Date: Wed, 14 Mar 2018 09:58:08 -0400 Subject: [PATCH] bugfix: cgroup-parent must be abs Signed-off-by: Michael Wan --- daemon/mgr/container.go | 5 +++++ test/cli_run_test.go | 18 +++++++++++++++++- 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/daemon/mgr/container.go b/daemon/mgr/container.go index 0d55e8f51..ff52460df 100644 --- a/daemon/mgr/container.go +++ b/daemon/mgr/container.go @@ -486,7 +486,12 @@ func (mgr *ContainerManager) Start(ctx context.Context, id, detachKeys string) ( cgroupsParent = mgr.Config.CgroupParent } + // cgroupsPath must be absolute path if cgroupsParent != "" { + if !filepath.IsAbs(cgroupsParent) { + cgroupsParent = filepath.Clean("/" + cgroupsParent) + } + s.Linux.CgroupsPath = filepath.Join(cgroupsParent, c.ID()) } diff --git a/test/cli_run_test.go b/test/cli_run_test.go index 0cddc0bda..71ac3a078 100644 --- a/test/cli_run_test.go +++ b/test/cli_run_test.go @@ -732,7 +732,7 @@ func testRunWithCgroupParent(c *check.C, cgroupParent, name string) { file := "/sys/fs/cgroup/memory/" + cgroupParent + "/" + containerID + "/memory.limit_in_bytes" if _, err := os.Stat(file); err != nil { - c.Fatalf("container %s cgroup mountpoint not exists", containerID) + c.Fatalf("container %s cgroup mountpoint not exists", name) } out, err := exec.Command("cat", file).Output() @@ -745,3 +745,19 @@ func testRunWithCgroupParent(c *check.C, cgroupParent, name string) { } } + +// TestRunInvalidCgroupParent checks that a specially-crafted cgroup parent doesn't cause Docker to crash or start modifying /. +func (suite *PouchRunSuite) TestRunInvalidCgroupParent(c *check.C) { + testRunInvalidCgroupParent(c, "../../../../../../../../SHOULD_NOT_EXIST", "SHOULD_NOT_EXIST", "cgroup-invalid-test") + + testRunInvalidCgroupParent(c, "/../../../../../../../../SHOULD_NOT_EXIST", "/SHOULD_NOT_EXIST", "cgroup-absolute-invalid-test") +} + +func testRunInvalidCgroupParent(c *check.C, cgroupParent, cleanCgroupParent, name string) { + command.PouchRun("run", "-m", "300M", "--cgroup-parent", cgroupParent, "--name", name, busyboxImage, "cat", "/proc/self/cgroup").Assert(c, icmd.Success) + + // We expect "/SHOULD_NOT_EXIST" to not exist. If not, we have a security issue. + if _, err := os.Stat("/SHOULD_NOT_EXIST"); err == nil || !os.IsNotExist(err) { + c.Fatalf("SECURITY: --cgroup-parent with ../../ relative paths cause files to be created in the host (this is bad) !!") + } +}