From bf899fef451956be4abd63de6d6141d9f9096a02 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 31 Dec 2015 11:05:45 +1100 Subject: [PATCH] cgroups: fs: fix cgroup.Parent path sanitisation Properly sanitise the --cgroup-parent path, to avoid potential issues (as it starts creating directories and writing to files as root). In addition, fix an infinite recursion due to incomplete base cases. It might be a good idea to move pathClean to a separate library (which deals with path safety concerns, so all of runC and Docker can take advantage of it). Signed-off-by: Aleksa Sarai --- libcontainer/cgroups/fs/apply_raw.go | 27 +++++++++++++++++++++++++++ libcontainer/cgroups/fs/cpuset.go | 5 +++++ 2 files changed, 32 insertions(+) diff --git a/libcontainer/cgroups/fs/apply_raw.go b/libcontainer/cgroups/fs/apply_raw.go index 0c4d207e..7d2da2dc 100644 --- a/libcontainer/cgroups/fs/apply_raw.go +++ b/libcontainer/cgroups/fs/apply_raw.go @@ -230,12 +230,39 @@ func (m *Manager) GetPids() ([]int, error) { return cgroups.GetPids(dir) } +// pathClean makes a path safe for use with filepath.Join. This is done by not +// only cleaning the path, but also (if the path is relative) adding a leading +// '/' and cleaning it (then removing the leading '/'). This ensures that a +// path resulting from prepending another path will always resolve to lexically +// be a subdirectory of the prefixed path. This is all done lexically, so paths +// that include symlinks won't be safe as a result of using pathClean. +func pathClean(path string) string { + // Ensure that all paths are cleaned (especially problematic ones like + // "/../../../../../" which can cause lots of issues). + path = filepath.Clean(path) + + // If the path isn't absolute, we need to do more processing to fix paths + // such as "../../../..//some/path". We also shouldn't convert absolute + // paths to relative ones. + if !filepath.IsAbs(path) { + path = filepath.Clean(string(os.PathSeparator) + path) + // This can't fail, as (by definition) all paths are relative to root. + path, _ = filepath.Rel(string(os.PathSeparator), path) + } + + // Clean the path again for good measure. + return filepath.Clean(path) +} + func getCgroupData(c *configs.Cgroup, pid int) (*cgroupData, error) { root, err := getCgroupRoot() if err != nil { return nil, err } + // Clean the parent slice path. + c.Parent = pathClean(c.Parent) + return &cgroupData{ root: root, parent: c.Parent, diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index b7632108..8daacfc6 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -4,6 +4,7 @@ package fs import ( "bytes" + "fmt" "io/ioutil" "os" "path/filepath" @@ -95,6 +96,10 @@ func (s *CpusetGroup) ensureParent(current, root string) error { if filepath.Clean(parent) == root { return nil } + // Avoid infinite recursion. + if parent == current { + return fmt.Errorf("cpuset: cgroup parent path outside cgroup root") + } if err := s.ensureParent(parent, root); err != nil { return err }