mirror of
https://github.com/moby/moby.git
synced 2022-11-09 12:21:53 -05:00
Merge pull request #5720 from cyphar/5656-cp-absolute-paths
Ensure `docker cp` cannot traverse outside container rootfs
This commit is contained in:
commit
4af465fccf
4 changed files with 235 additions and 13 deletions
1
AUTHORS
1
AUTHORS
|
@ -6,6 +6,7 @@
|
|||
Aanand Prasad <aanand.prasad@gmail.com>
|
||||
Aaron Feng <aaron.feng@gmail.com>
|
||||
Abel Muiño <amuino@gmail.com>
|
||||
Aleksa Sarai <cyphar@cyphar.com>
|
||||
Alexander Larsson <alexl@redhat.com>
|
||||
Alexey Shamrin <shamrin@gmail.com>
|
||||
Alex Gaynor <alex.gaynor@gmail.com>
|
||||
|
|
|
@ -9,6 +9,7 @@ import (
|
|||
"log"
|
||||
"os"
|
||||
"path"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"sync"
|
||||
"syscall"
|
||||
|
@ -89,7 +90,7 @@ func (container *Container) Inject(file io.Reader, pth string) error {
|
|||
defer container.Unmount()
|
||||
|
||||
// Return error if path exists
|
||||
destPath := path.Join(container.basefs, pth)
|
||||
destPath := container.getResourcePath(pth)
|
||||
if _, err := os.Stat(destPath); err == nil {
|
||||
// Since err is nil, the path could be stat'd and it exists
|
||||
return fmt.Errorf("%s exists", pth)
|
||||
|
@ -101,7 +102,7 @@ func (container *Container) Inject(file io.Reader, pth string) error {
|
|||
}
|
||||
|
||||
// Make sure the directory exists
|
||||
if err := os.MkdirAll(path.Join(container.basefs, path.Dir(pth)), 0755); err != nil {
|
||||
if err := os.MkdirAll(container.getResourcePath(path.Dir(pth)), 0755); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
|
@ -170,6 +171,16 @@ func (container *Container) WriteHostConfig() (err error) {
|
|||
return ioutil.WriteFile(container.hostConfigPath(), data, 0666)
|
||||
}
|
||||
|
||||
func (container *Container) getResourcePath(path string) string {
|
||||
cleanPath := filepath.Join("/", path)
|
||||
return filepath.Join(container.basefs, cleanPath)
|
||||
}
|
||||
|
||||
func (container *Container) getRootResourcePath(path string) string {
|
||||
cleanPath := filepath.Join("/", path)
|
||||
return filepath.Join(container.root, cleanPath)
|
||||
}
|
||||
|
||||
func populateCommand(c *Container, env []string) error {
|
||||
var (
|
||||
en *execdriver.Network
|
||||
|
@ -345,7 +356,7 @@ func (container *Container) StderrLogPipe() io.ReadCloser {
|
|||
}
|
||||
|
||||
func (container *Container) buildHostnameFile() error {
|
||||
container.HostnamePath = path.Join(container.root, "hostname")
|
||||
container.HostnamePath = container.getRootResourcePath("hostname")
|
||||
if container.Config.Domainname != "" {
|
||||
return ioutil.WriteFile(container.HostnamePath, []byte(fmt.Sprintf("%s.%s\n", container.Config.Hostname, container.Config.Domainname)), 0644)
|
||||
}
|
||||
|
@ -357,7 +368,7 @@ func (container *Container) buildHostnameAndHostsFiles(IP string) error {
|
|||
return err
|
||||
}
|
||||
|
||||
container.HostsPath = path.Join(container.root, "hosts")
|
||||
container.HostsPath = container.getRootResourcePath("hosts")
|
||||
|
||||
extraContent := make(map[string]string)
|
||||
|
||||
|
@ -675,7 +686,7 @@ func (container *Container) Unmount() error {
|
|||
}
|
||||
|
||||
func (container *Container) logPath(name string) string {
|
||||
return path.Join(container.root, fmt.Sprintf("%s-%s.log", container.ID, name))
|
||||
return container.getRootResourcePath(fmt.Sprintf("%s-%s.log", container.ID, name))
|
||||
}
|
||||
|
||||
func (container *Container) ReadLog(name string) (io.Reader, error) {
|
||||
|
@ -683,11 +694,11 @@ func (container *Container) ReadLog(name string) (io.Reader, error) {
|
|||
}
|
||||
|
||||
func (container *Container) hostConfigPath() string {
|
||||
return path.Join(container.root, "hostconfig.json")
|
||||
return container.getRootResourcePath("hostconfig.json")
|
||||
}
|
||||
|
||||
func (container *Container) jsonPath() string {
|
||||
return path.Join(container.root, "config.json")
|
||||
return container.getRootResourcePath("config.json")
|
||||
}
|
||||
|
||||
// This method must be exported to be used from the lxc template
|
||||
|
@ -746,8 +757,10 @@ func (container *Container) Copy(resource string) (io.ReadCloser, error) {
|
|||
if err := container.Mount(); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
||||
var filter []string
|
||||
basePath := path.Join(container.basefs, resource)
|
||||
|
||||
basePath := container.getResourcePath(resource)
|
||||
stat, err := os.Stat(basePath)
|
||||
if err != nil {
|
||||
container.Unmount()
|
||||
|
@ -845,7 +858,7 @@ func (container *Container) setupContainerDns() error {
|
|||
} else if len(daemon.config.DnsSearch) > 0 {
|
||||
dnsSearch = daemon.config.DnsSearch
|
||||
}
|
||||
container.ResolvConfPath = path.Join(container.root, "resolv.conf")
|
||||
container.ResolvConfPath = container.getRootResourcePath("resolv.conf")
|
||||
return resolvconf.Build(container.ResolvConfPath, dns, dnsSearch)
|
||||
} else {
|
||||
container.ResolvConfPath = "/etc/resolv.conf"
|
||||
|
@ -983,12 +996,12 @@ func (container *Container) setupWorkingDirectory() error {
|
|||
if container.Config.WorkingDir != "" {
|
||||
container.Config.WorkingDir = path.Clean(container.Config.WorkingDir)
|
||||
|
||||
pthInfo, err := os.Stat(path.Join(container.basefs, container.Config.WorkingDir))
|
||||
pthInfo, err := os.Stat(container.getResourcePath(container.Config.WorkingDir))
|
||||
if err != nil {
|
||||
if !os.IsNotExist(err) {
|
||||
return err
|
||||
}
|
||||
if err := os.MkdirAll(path.Join(container.basefs, container.Config.WorkingDir), 0755); err != nil {
|
||||
if err := os.MkdirAll(container.getResourcePath(container.Config.WorkingDir), 0755); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
|
|
@ -94,11 +94,11 @@ func applyVolumesFrom(container *Container) error {
|
|||
if _, exists := container.Volumes[volPath]; exists {
|
||||
continue
|
||||
}
|
||||
stat, err := os.Stat(filepath.Join(c.basefs, volPath))
|
||||
stat, err := os.Stat(c.getResourcePath(volPath))
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if err := createIfNotExists(filepath.Join(container.basefs, volPath), stat.IsDir()); err != nil {
|
||||
if err := createIfNotExists(container.getResourcePath(volPath), stat.IsDir()); err != nil {
|
||||
return err
|
||||
}
|
||||
container.Volumes[volPath] = id
|
||||
|
|
208
integration-cli/docker_cli_cp_test.go
Normal file
208
integration-cli/docker_cli_cp_test.go
Normal file
|
@ -0,0 +1,208 @@
|
|||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
)
|
||||
|
||||
const (
|
||||
cpTestPathParent = "/some"
|
||||
cpTestPath = "/some/path"
|
||||
cpTestName = "test"
|
||||
cpFullPath = "/some/path/test"
|
||||
|
||||
cpContainerContents = "holla, i am the container"
|
||||
cpHostContents = "hello, i am the host"
|
||||
)
|
||||
|
||||
// Test for #5656
|
||||
// Check that garbage paths don't escape the container's rootfs
|
||||
func TestCpGarbagePath(t *testing.T) {
|
||||
out, exitCode, err := cmd(t, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath)
|
||||
if err != nil || exitCode != 0 {
|
||||
t.Fatal("failed to create a container", out, err)
|
||||
}
|
||||
|
||||
cleanedContainerID := stripTrailingCharacters(out)
|
||||
defer deleteContainer(cleanedContainerID)
|
||||
|
||||
out, _, err = cmd(t, "wait", cleanedContainerID)
|
||||
if err != nil || stripTrailingCharacters(out) != "0" {
|
||||
t.Fatal("failed to set up container", out, err)
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(cpTestPath, os.ModeDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
hostFile, err := os.Create(cpFullPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer hostFile.Close()
|
||||
defer os.RemoveAll(cpTestPathParent)
|
||||
|
||||
fmt.Fprintf(hostFile, "%s", cpHostContents)
|
||||
|
||||
tmpdir, err := ioutil.TempDir("", "docker-integration")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmpname := filepath.Join(tmpdir, cpTestName)
|
||||
defer os.RemoveAll(tmpdir)
|
||||
|
||||
path := filepath.Join("../../../../../../../../../../../../", cpFullPath)
|
||||
|
||||
_, _, err = cmd(t, "cp", cleanedContainerID+":"+path, tmpdir)
|
||||
if err != nil {
|
||||
t.Fatalf("couldn't copy from garbage path: %s:%s %s", cleanedContainerID, path, err)
|
||||
}
|
||||
|
||||
file, _ := os.Open(tmpname)
|
||||
defer file.Close()
|
||||
|
||||
test, err := ioutil.ReadAll(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if string(test) == cpHostContents {
|
||||
t.Errorf("output matched host file -- garbage path can escape container rootfs")
|
||||
}
|
||||
|
||||
if string(test) != cpContainerContents {
|
||||
t.Errorf("output doesn't match the input for garbage path")
|
||||
}
|
||||
|
||||
logDone("cp - garbage paths relative to container's rootfs")
|
||||
}
|
||||
|
||||
// Check that relative paths are relative to the container's rootfs
|
||||
func TestCpRelativePath(t *testing.T) {
|
||||
out, exitCode, err := cmd(t, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath)
|
||||
if err != nil || exitCode != 0 {
|
||||
t.Fatal("failed to create a container", out, err)
|
||||
}
|
||||
|
||||
cleanedContainerID := stripTrailingCharacters(out)
|
||||
defer deleteContainer(cleanedContainerID)
|
||||
|
||||
out, _, err = cmd(t, "wait", cleanedContainerID)
|
||||
if err != nil || stripTrailingCharacters(out) != "0" {
|
||||
t.Fatal("failed to set up container", out, err)
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(cpTestPath, os.ModeDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
hostFile, err := os.Create(cpFullPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer hostFile.Close()
|
||||
defer os.RemoveAll(cpTestPathParent)
|
||||
|
||||
fmt.Fprintf(hostFile, "%s", cpHostContents)
|
||||
|
||||
tmpdir, err := ioutil.TempDir("", "docker-integration")
|
||||
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmpname := filepath.Join(tmpdir, cpTestName)
|
||||
defer os.RemoveAll(tmpdir)
|
||||
|
||||
path, _ := filepath.Rel("/", cpFullPath)
|
||||
|
||||
_, _, err = cmd(t, "cp", cleanedContainerID+":"+path, tmpdir)
|
||||
if err != nil {
|
||||
t.Fatalf("couldn't copy from relative path: %s:%s %s", cleanedContainerID, path, err)
|
||||
}
|
||||
|
||||
file, _ := os.Open(tmpname)
|
||||
defer file.Close()
|
||||
|
||||
test, err := ioutil.ReadAll(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if string(test) == cpHostContents {
|
||||
t.Errorf("output matched host file -- relative path can escape container rootfs")
|
||||
}
|
||||
|
||||
if string(test) != cpContainerContents {
|
||||
t.Errorf("output doesn't match the input for relative path")
|
||||
}
|
||||
|
||||
logDone("cp - relative paths relative to container's rootfs")
|
||||
}
|
||||
|
||||
// Check that absolute paths are relative to the container's rootfs
|
||||
func TestCpAbsolutePath(t *testing.T) {
|
||||
out, exitCode, err := cmd(t, "run", "-d", "busybox", "/bin/sh", "-c", "mkdir -p '"+cpTestPath+"' && echo -n '"+cpContainerContents+"' > "+cpFullPath)
|
||||
if err != nil || exitCode != 0 {
|
||||
t.Fatal("failed to create a container", out, err)
|
||||
}
|
||||
|
||||
cleanedContainerID := stripTrailingCharacters(out)
|
||||
defer deleteContainer(cleanedContainerID)
|
||||
|
||||
out, _, err = cmd(t, "wait", cleanedContainerID)
|
||||
if err != nil || stripTrailingCharacters(out) != "0" {
|
||||
t.Fatal("failed to set up container", out, err)
|
||||
}
|
||||
|
||||
if err := os.MkdirAll(cpTestPath, os.ModeDir); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
hostFile, err := os.Create(cpFullPath)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
defer hostFile.Close()
|
||||
defer os.RemoveAll(cpTestPathParent)
|
||||
|
||||
fmt.Fprintf(hostFile, "%s", cpHostContents)
|
||||
|
||||
tmpdir, err := ioutil.TempDir("", "docker-integration")
|
||||
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
tmpname := filepath.Join(tmpdir, cpTestName)
|
||||
defer os.RemoveAll(tmpdir)
|
||||
|
||||
path := cpFullPath
|
||||
|
||||
_, _, err = cmd(t, "cp", cleanedContainerID+":"+path, tmpdir)
|
||||
if err != nil {
|
||||
t.Fatalf("couldn't copy from absolute path: %s:%s %s", cleanedContainerID, path, err)
|
||||
}
|
||||
|
||||
file, _ := os.Open(tmpname)
|
||||
defer file.Close()
|
||||
|
||||
test, err := ioutil.ReadAll(file)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if string(test) == cpHostContents {
|
||||
t.Errorf("output matched host file -- absolute path can escape container rootfs")
|
||||
}
|
||||
|
||||
if string(test) != cpContainerContents {
|
||||
t.Errorf("output doesn't match the input for absolute path")
|
||||
}
|
||||
|
||||
logDone("cp - absolute paths relative to container's rootfs")
|
||||
}
|
Loading…
Reference in a new issue