diff --git a/container/archive.go b/container/archive.go index 267ad18d7e..e22a001c59 100644 --- a/container/archive.go +++ b/container/archive.go @@ -5,6 +5,7 @@ import ( "path/filepath" "github.com/docker/docker/pkg/archive" + "github.com/docker/docker/pkg/system" "github.com/docker/engine-api/types" ) @@ -13,6 +14,12 @@ import ( // the absolute path to the resource relative to the container's rootfs, and // an error if the path points to outside the container's rootfs. func (container *Container) ResolvePath(path string) (resolvedPath, absPath string, err error) { + // Check if a drive letter supplied, it must be the system drive. No-op except on Windows + path, err = system.CheckSystemDriveAndRemoveDriveLetter(path) + if err != nil { + return "", "", err + } + // Consider the given path as an absolute path in the container. absPath = archive.PreserveTrailingDotOrSeparator(filepath.Join(string(filepath.Separator), path), path) diff --git a/daemon/archive.go b/daemon/archive.go index 5cf6985210..b2221560a3 100644 --- a/daemon/archive.go +++ b/daemon/archive.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/pkg/chrootarchive" "github.com/docker/docker/pkg/idtools" "github.com/docker/docker/pkg/ioutils" + "github.com/docker/docker/pkg/system" "github.com/docker/engine-api/types" ) @@ -188,6 +189,12 @@ func (daemon *Daemon) containerExtractToDir(container *container.Container, path return err } + // Check if a drive letter supplied, it must be the system drive. No-op except on Windows + path, err = system.CheckSystemDriveAndRemoveDriveLetter(path) + if err != nil { + return err + } + // The destination path needs to be resolved to a host path, with all // symbolic links followed in the scope of the container's rootfs. Note // that we do not use `container.ResolvePath(path)` here because we need diff --git a/daemon/archive_unix.go b/daemon/archive_unix.go index fcea13c929..47666fe5e8 100644 --- a/daemon/archive_unix.go +++ b/daemon/archive_unix.go @@ -3,9 +3,10 @@ package daemon import ( - "github.com/docker/docker/container" "os" "path/filepath" + + "github.com/docker/docker/container" ) // checkIfPathIsInAVolume checks if the path is in a volume. If it is, it diff --git a/pkg/system/path_unix.go b/pkg/system/path_unix.go index 1b6cc9cbd9..c607c4db09 100644 --- a/pkg/system/path_unix.go +++ b/pkg/system/path_unix.go @@ -6,3 +6,9 @@ package system // executables. Each directory is separated from the next by a colon // ':' character . const DefaultPathEnv = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" + +// CheckSystemDriveAndRemoveDriveLetter verifies that a path, if it includes a drive letter, +// is the system drive. This is a no-op on Linux. +func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) { + return path, nil +} diff --git a/pkg/system/path_windows.go b/pkg/system/path_windows.go index 09e7f89fed..cbfe2c1576 100644 --- a/pkg/system/path_windows.go +++ b/pkg/system/path_windows.go @@ -2,6 +2,36 @@ package system +import ( + "fmt" + "path/filepath" + "strings" +) + // DefaultPathEnv is deliberately empty on Windows as the default path will be set by // the container. Docker has no context of what the default path should be. const DefaultPathEnv = "" + +// CheckSystemDriveAndRemoveDriveLetter verifies and manipulates a Windows path. +// This is used, for example, when validating a user provided path in docker cp. +// If a drive letter is supplied, it must be the system drive. The drive letter +// is always removed. Also, it translates it to OS semantics (IOW / to \). We +// need the path in this syntax so that it can ultimately be contatenated with +// a Windows long-path which doesn't support drive-letters. Examples: +// C: --> Fail +// C:\ --> \ +// a --> a +// /a --> \a +// d:\ --> Fail +func CheckSystemDriveAndRemoveDriveLetter(path string) (string, error) { + if len(path) == 2 && string(path[1]) == ":" { + return "", fmt.Errorf("No relative path specified in %q", path) + } + if !filepath.IsAbs(path) || len(path) < 2 { + return filepath.FromSlash(path), nil + } + if string(path[1]) == ":" && !strings.EqualFold(string(path[0]), "c") { + return "", fmt.Errorf("The specified path is not on the system drive (C:)") + } + return filepath.FromSlash(path[2:]), nil +} diff --git a/pkg/system/path_windows_test.go b/pkg/system/path_windows_test.go new file mode 100644 index 0000000000..eccb26aaea --- /dev/null +++ b/pkg/system/path_windows_test.go @@ -0,0 +1,78 @@ +// +build windows + +package system + +import "testing" + +// TestCheckSystemDriveAndRemoveDriveLetter tests CheckSystemDriveAndRemoveDriveLetter +func TestCheckSystemDriveAndRemoveDriveLetter(t *testing.T) { + // Fails if not C drive. + path, err := CheckSystemDriveAndRemoveDriveLetter(`d:\`) + if err == nil || (err != nil && err.Error() != "The specified path is not on the system drive (C:)") { + t.Fatalf("Expected error for d:") + } + + // Single character is unchanged + if path, err = CheckSystemDriveAndRemoveDriveLetter("z"); err != nil { + t.Fatalf("Single character should pass") + } + if path != "z" { + t.Fatalf("Single character should be unchanged") + } + + // Two characters without colon is unchanged + if path, err = CheckSystemDriveAndRemoveDriveLetter("AB"); err != nil { + t.Fatalf("2 characters without colon should pass") + } + if path != "AB" { + t.Fatalf("2 characters without colon should be unchanged") + } + + // Abs path without drive letter + if path, err = CheckSystemDriveAndRemoveDriveLetter(`\l`); err != nil { + t.Fatalf("abs path no drive letter should pass") + } + if path != `\l` { + t.Fatalf("abs path without drive letter should be unchanged") + } + + // Abs path without drive letter, linux style + if path, err = CheckSystemDriveAndRemoveDriveLetter(`/l`); err != nil { + t.Fatalf("abs path no drive letter linux style should pass") + } + if path != `\l` { + t.Fatalf("abs path without drive letter linux failed %s", path) + } + + // Drive-colon should be stripped + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:\`); err != nil { + t.Fatalf("An absolute path should pass") + } + if path != `\` { + t.Fatalf(`An absolute path should have been shortened to \ %s`, path) + } + + // Verify with a linux-style path + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:/`); err != nil { + t.Fatalf("An absolute path should pass") + } + if path != `\` { + t.Fatalf(`A linux style absolute path should have been shortened to \ %s`, path) + } + + // Failure on c: + if path, err = CheckSystemDriveAndRemoveDriveLetter(`c:`); err == nil { + t.Fatalf("c: should fail") + } + if err.Error() != `No relative path specified in "c:"` { + t.Fatalf(path, err) + } + + // Failure on d: + if path, err = CheckSystemDriveAndRemoveDriveLetter(`d:`); err == nil { + t.Fatalf("c: should fail") + } + if err.Error() != `No relative path specified in "d:"` { + t.Fatalf(path, err) + } +}