From 177fad15b5ccefc7490701ab6b383824e619f901 Mon Sep 17 00:00:00 2001 From: Brian Goff Date: Thu, 30 May 2019 11:15:09 -0700 Subject: [PATCH] Pass root to chroot to for chroot Untar This is useful for preventing CVE-2018-15664 where a malicious container process can take advantage of a race on symlink resolution/sanitization. Before this change chrootarchive would chroot to the destination directory which is attacker controlled. With this patch we always chroot to the container's root which is not attacker controlled. Signed-off-by: Brian Goff Origin: upstream, https://github.com/moby/moby/pull/39292 Gbp-Pq: Name cve-2018-15664-01-pass-root-to-chroot-to-for-chroot-untar.patch --- engine/daemon/archive.go | 7 +++-- engine/pkg/chrootarchive/archive.go | 24 ++++++++++++--- engine/pkg/chrootarchive/archive_unix.go | 34 ++++++++++++++++++--- engine/pkg/chrootarchive/archive_windows.go | 2 +- 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/engine/daemon/archive.go b/engine/daemon/archive.go index 9c7971b5..9f56ca75 100644 --- a/engine/daemon/archive.go +++ b/engine/daemon/archive.go @@ -31,11 +31,12 @@ type archiver interface { } // helper functions to extract or archive -func extractArchive(i interface{}, src io.Reader, dst string, opts *archive.TarOptions) error { +func extractArchive(i interface{}, src io.Reader, dst string, opts *archive.TarOptions, root string) error { if ea, ok := i.(extractor); ok { return ea.ExtractArchive(src, dst, opts) } - return chrootarchive.Untar(src, dst, opts) + + return chrootarchive.UntarWithRoot(src, dst, opts, root) } func archivePath(i interface{}, src string, opts *archive.TarOptions) (io.ReadCloser, error) { @@ -367,7 +368,7 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path } } - if err := extractArchive(driver, content, resolvedPath, options); err != nil { + if err := extractArchive(driver, content, resolvedPath, options, container.BaseFS.Path()); err != nil { return err } diff --git a/engine/pkg/chrootarchive/archive.go b/engine/pkg/chrootarchive/archive.go index 2d9d6628..7ebca377 100644 --- a/engine/pkg/chrootarchive/archive.go +++ b/engine/pkg/chrootarchive/archive.go @@ -27,18 +27,34 @@ func NewArchiver(idMapping *idtools.IdentityMapping) *archive.Archiver { // The archive may be compressed with one of the following algorithms: // identity (uncompressed), gzip, bzip2, xz. func Untar(tarArchive io.Reader, dest string, options *archive.TarOptions) error { - return untarHandler(tarArchive, dest, options, true) + return untarHandler(tarArchive, dest, options, true, dest) +} + +// UntarWithRoot is the same as `Untar`, but allows you to pass in a root directory +// The root directory is the directory that will be chrooted to. +// `dest` must be a path within `root`, if it is not an error will be returned. +// +// `root` should set to a directory which is not controlled by any potentially +// malicious process. +// +// This should be used to prevent a potential attacker from manipulating `dest` +// such that it would provide access to files outside of `dest` through things +// like symlinks. Normally `ResolveSymlinksInScope` would handle this, however +// sanitizing symlinks in this manner is inherrently racey: +// ref: CVE-2018-15664 +func UntarWithRoot(tarArchive io.Reader, dest string, options *archive.TarOptions, root string) error { + return untarHandler(tarArchive, dest, options, true, root) } // UntarUncompressed reads a stream of bytes from `archive`, parses it as a tar archive, // and unpacks it into the directory at `dest`. // The archive must be an uncompressed stream. func UntarUncompressed(tarArchive io.Reader, dest string, options *archive.TarOptions) error { - return untarHandler(tarArchive, dest, options, false) + return untarHandler(tarArchive, dest, options, false, dest) } // Handler for teasing out the automatic decompression -func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions, decompress bool) error { +func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions, decompress bool, root string) error { if tarArchive == nil { return fmt.Errorf("Empty archive") } @@ -69,5 +85,5 @@ func untarHandler(tarArchive io.Reader, dest string, options *archive.TarOptions r = decompressedArchive } - return invokeUnpack(r, dest, options) + return invokeUnpack(r, dest, options, root) } diff --git a/engine/pkg/chrootarchive/archive_unix.go b/engine/pkg/chrootarchive/archive_unix.go index 5df8afd6..96f07c4b 100644 --- a/engine/pkg/chrootarchive/archive_unix.go +++ b/engine/pkg/chrootarchive/archive_unix.go @@ -10,6 +10,7 @@ import ( "io" "io/ioutil" "os" + "path/filepath" "runtime" "github.com/docker/docker/pkg/archive" @@ -30,11 +31,21 @@ func untar() { fatal(err) } - if err := chroot(flag.Arg(0)); err != nil { + dst := flag.Arg(0) + var root string + if len(flag.Args()) > 1 { + root = flag.Arg(1) + } + + if root == "" { + root = dst + } + + if err := chroot(root); err != nil { fatal(err) } - if err := archive.Unpack(os.Stdin, "/", options); err != nil { + if err := archive.Unpack(os.Stdin, dst, options); err != nil { fatal(err) } // fully consume stdin in case it is zero padded @@ -45,7 +56,7 @@ func untar() { os.Exit(0) } -func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions) error { +func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.TarOptions, root string) error { // We can't pass a potentially large exclude list directly via cmd line // because we easily overrun the kernel's max argument/environment size @@ -57,7 +68,21 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T return fmt.Errorf("Untar pipe failure: %v", err) } - cmd := reexec.Command("docker-untar", dest) + if root != "" { + relDest, err := filepath.Rel(root, dest) + if err != nil { + return err + } + if relDest == "." { + relDest = "/" + } + if relDest[0] != '/' { + relDest = "/" + relDest + } + dest = relDest + } + + cmd := reexec.Command("docker-untar", dest, root) cmd.Stdin = decompressedArchive cmd.ExtraFiles = append(cmd.ExtraFiles, r) @@ -69,6 +94,7 @@ func invokeUnpack(decompressedArchive io.Reader, dest string, options *archive.T w.Close() return fmt.Errorf("Untar error on re-exec cmd: %v", err) } + //write the options to the pipe for the untar exec to read if err := json.NewEncoder(w).Encode(options); err != nil { w.Close() diff --git a/engine/pkg/chrootarchive/archive_windows.go b/engine/pkg/chrootarchive/archive_windows.go index f2973132..bd5712c5 100644 --- a/engine/pkg/chrootarchive/archive_windows.go +++ b/engine/pkg/chrootarchive/archive_windows.go @@ -14,7 +14,7 @@ func chroot(path string) error { func invokeUnpack(decompressedArchive io.ReadCloser, dest string, - options *archive.TarOptions) error { + options *archive.TarOptions, root string) error { // Windows is different to Linux here because Windows does not support // chroot. Hence there is no point sandboxing a chrooted process to // do the unpack. We call inline instead within the daemon process. -- 2.30.2