From b0e75888738f6e427b545922a195837c00158749 Mon Sep 17 00:00:00 2001 From: Simon Ferquel Date: Fri, 24 Mar 2017 12:07:08 +0100 Subject: [PATCH] Test and fix forbidden path for COPY --from on Windows Paths resolving to c:\ or c:\windows are forbidden Replaced the obscure (and non-working) regex with a simple case insensitive comparison to the black listed paths (we should forbid c:\, c:\windows but not d:\) Also, add a test ensuring paths are case insensitive on windows Also, made sure existing multi-staged build tests pass on windows Signed-off-by: Simon Ferquel --- builder/dockerfile/internals.go | 23 ++++-- integration-cli/docker_cli_build_test.go | 91 ++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/builder/dockerfile/internals.go b/builder/dockerfile/internals.go index 80385c4f66..0b77bb66e2 100644 --- a/builder/dockerfile/internals.go +++ b/builder/dockerfile/internals.go @@ -13,7 +13,6 @@ import ( "net/url" "os" "path/filepath" - "regexp" "runtime" "sort" "strings" @@ -298,16 +297,30 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) { return &builder.HashedFileInfo{FileInfo: builder.PathFileInfo{FileInfo: tmpFileSt, FilePath: tmpFileName}, FileHash: hash}, nil } +var windowsBlacklist = map[string]bool{ + "c:\\": true, + "c:\\windows": true, +} + func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, imageSource *imageMount) ([]copyInfo, error) { // Work in daemon-specific OS filepath semantics origPath = filepath.FromSlash(origPath) - // validate windows paths from other images if imageSource != nil && runtime.GOOS == "windows" { - forbid := regexp.MustCompile("(?i)^" + string(os.PathSeparator) + "?(windows(" + string(os.PathSeparator) + ".+)?)?$") - if p := filepath.Clean(origPath); p == "." || forbid.MatchString(p) { - return nil, errors.Errorf("copy from %s is not allowed on windows", origPath) + p := strings.ToLower(filepath.Clean(origPath)) + if !filepath.IsAbs(p) { + if filepath.VolumeName(p) != "" { + if p[len(p)-2:] == ":." { // case where clean returns weird c:. paths + p = p[:len(p)-1] + } + p += "\\" + } else { + p = filepath.Join("c:\\", p) + } + } + if _, blacklisted := windowsBlacklist[p]; blacklisted { + return nil, errors.New("copy from c:\\ or c:\\windows is not allowed on windows") } } diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 81d08e5419..269659df74 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/integration-cli/docker_cli_build_test.go @@ -6024,6 +6024,97 @@ func (s *DockerTrustSuite) TestCopyFromTrustedBuild(c *check.C) { dockerCmdWithResult("run", name, "cat", "bar").Assert(c, icmd.Expected{Out: "ok"}) } +func (s *DockerSuite) TestBuildCopyFromPreviousFromWindows(c *check.C) { + testRequires(c, DaemonIsWindows) + dockerfile := ` + FROM ` + testEnv.MinimalBaseImage() + ` + COPY foo c:\\bar` + ctx := fakeContext(c, dockerfile, map[string]string{ + "Dockerfile": dockerfile, + "foo": "abc", + }) + defer ctx.Close() + + result := buildImage("build1", withExternalBuildContext(ctx)) + result.Assert(c, icmd.Success) + + dockerfile = ` + FROM build1:latest + FROM ` + testEnv.MinimalBaseImage() + ` + COPY --from=0 c:\\bar / + COPY foo /` + ctx = fakeContext(c, dockerfile, map[string]string{ + "Dockerfile": dockerfile, + "foo": "def", + }) + defer ctx.Close() + + result = buildImage("build2", withExternalBuildContext(ctx)) + result.Assert(c, icmd.Success) + + out, _ := dockerCmd(c, "run", "build2", "cmd.exe", "/s", "/c", "type", "c:\\bar") + c.Assert(strings.TrimSpace(out), check.Equals, "abc") + out, _ = dockerCmd(c, "run", "build2", "cmd.exe", "/s", "/c", "type", "c:\\foo") + c.Assert(strings.TrimSpace(out), check.Equals, "def") +} + +func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) { + testRequires(c, DaemonIsWindows) + dockerfile := ` + FROM ` + testEnv.MinimalBaseImage() + ` + FROM ` + testEnv.MinimalBaseImage() + ` + COPY --from=0 %s c:\\oscopy + ` + exp := icmd.Expected{ + ExitCode: 1, + Err: "copy from c:\\ or c:\\windows is not allowed on windows", + } + buildImage("testforbidsystempaths1", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\"))).Assert(c, exp) + buildImage("testforbidsystempaths2", build.WithDockerfile(fmt.Sprintf(dockerfile, "C:\\\\"))).Assert(c, exp) + buildImage("testforbidsystempaths3", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\windows"))).Assert(c, exp) + buildImage("testforbidsystempaths4", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\wInDows"))).Assert(c, exp) +} + +func (s *DockerSuite) TestBuildCopyFromForbidWindowsRelativePaths(c *check.C) { + testRequires(c, DaemonIsWindows) + dockerfile := ` + FROM ` + testEnv.MinimalBaseImage() + ` + FROM ` + testEnv.MinimalBaseImage() + ` + COPY --from=0 %s c:\\oscopy + ` + exp := icmd.Expected{ + ExitCode: 1, + Err: "copy from c:\\ or c:\\windows is not allowed on windows", + } + buildImage("testforbidsystempaths1", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:"))).Assert(c, exp) + buildImage("testforbidsystempaths2", build.WithDockerfile(fmt.Sprintf(dockerfile, "."))).Assert(c, exp) + buildImage("testforbidsystempaths3", build.WithDockerfile(fmt.Sprintf(dockerfile, "..\\\\"))).Assert(c, exp) + buildImage("testforbidsystempaths4", build.WithDockerfile(fmt.Sprintf(dockerfile, ".\\\\windows"))).Assert(c, exp) + buildImage("testforbidsystempaths5", build.WithDockerfile(fmt.Sprintf(dockerfile, "\\\\windows"))).Assert(c, exp) +} + +func (s *DockerSuite) TestBuildCopyFromWindowsIsCaseInsensitive(c *check.C) { + testRequires(c, DaemonIsWindows) + dockerfile := ` + FROM ` + testEnv.MinimalBaseImage() + ` + COPY foo / + FROM ` + testEnv.MinimalBaseImage() + ` + COPY --from=0 c:\\fOo c:\\copied + RUN type c:\\copied + ` + ctx := fakeContext(c, dockerfile, map[string]string{ + "Dockerfile": dockerfile, + "foo": "hello world", + }) + defer ctx.Close() + exp := icmd.Expected{ + ExitCode: 0, + Out: "hello world", + } + result := buildImage("copyfrom-windows-insensitive", withExternalBuildContext(ctx)) + result.Assert(c, exp) +} + // TestBuildOpaqueDirectory tests that a build succeeds which // creates opaque directories. // See https://github.com/docker/docker/issues/25244