Browse code

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 <simon.ferquel@docker.com>

Simon Ferquel authored on 2017/03/24 20:07:08
Showing 2 changed files
... ...
@@ -13,7 +13,6 @@ import (
13 13
 	"net/url"
14 14
 	"os"
15 15
 	"path/filepath"
16
-	"regexp"
17 16
 	"runtime"
18 17
 	"sort"
19 18
 	"strings"
... ...
@@ -298,16 +297,30 @@ func (b *Builder) download(srcURL string) (fi builder.FileInfo, err error) {
298 298
 	return &builder.HashedFileInfo{FileInfo: builder.PathFileInfo{FileInfo: tmpFileSt, FilePath: tmpFileName}, FileHash: hash}, nil
299 299
 }
300 300
 
301
+var windowsBlacklist = map[string]bool{
302
+	"c:\\":        true,
303
+	"c:\\windows": true,
304
+}
305
+
301 306
 func (b *Builder) calcCopyInfo(cmdName, origPath string, allowLocalDecompression, allowWildcards bool, imageSource *imageMount) ([]copyInfo, error) {
302 307
 
303 308
 	// Work in daemon-specific OS filepath semantics
304 309
 	origPath = filepath.FromSlash(origPath)
305
-
306 310
 	// validate windows paths from other images
307 311
 	if imageSource != nil && runtime.GOOS == "windows" {
308
-		forbid := regexp.MustCompile("(?i)^" + string(os.PathSeparator) + "?(windows(" + string(os.PathSeparator) + ".+)?)?$")
309
-		if p := filepath.Clean(origPath); p == "." || forbid.MatchString(p) {
310
-			return nil, errors.Errorf("copy from %s is not allowed on windows", origPath)
312
+		p := strings.ToLower(filepath.Clean(origPath))
313
+		if !filepath.IsAbs(p) {
314
+			if filepath.VolumeName(p) != "" {
315
+				if p[len(p)-2:] == ":." { // case where clean returns weird c:. paths
316
+					p = p[:len(p)-1]
317
+				}
318
+				p += "\\"
319
+			} else {
320
+				p = filepath.Join("c:\\", p)
321
+			}
322
+		}
323
+		if _, blacklisted := windowsBlacklist[p]; blacklisted {
324
+			return nil, errors.New("copy from c:\\ or c:\\windows is not allowed on windows")
311 325
 		}
312 326
 	}
313 327
 
... ...
@@ -6024,6 +6024,97 @@ func (s *DockerTrustSuite) TestCopyFromTrustedBuild(c *check.C) {
6024 6024
 	dockerCmdWithResult("run", name, "cat", "bar").Assert(c, icmd.Expected{Out: "ok"})
6025 6025
 }
6026 6026
 
6027
+func (s *DockerSuite) TestBuildCopyFromPreviousFromWindows(c *check.C) {
6028
+	testRequires(c, DaemonIsWindows)
6029
+	dockerfile := `
6030
+		FROM ` + testEnv.MinimalBaseImage() + `
6031
+		COPY foo c:\\bar`
6032
+	ctx := fakeContext(c, dockerfile, map[string]string{
6033
+		"Dockerfile": dockerfile,
6034
+		"foo":        "abc",
6035
+	})
6036
+	defer ctx.Close()
6037
+
6038
+	result := buildImage("build1", withExternalBuildContext(ctx))
6039
+	result.Assert(c, icmd.Success)
6040
+
6041
+	dockerfile = `
6042
+		FROM build1:latest
6043
+    	FROM ` + testEnv.MinimalBaseImage() + `
6044
+		COPY --from=0 c:\\bar /
6045
+		COPY foo /`
6046
+	ctx = fakeContext(c, dockerfile, map[string]string{
6047
+		"Dockerfile": dockerfile,
6048
+		"foo":        "def",
6049
+	})
6050
+	defer ctx.Close()
6051
+
6052
+	result = buildImage("build2", withExternalBuildContext(ctx))
6053
+	result.Assert(c, icmd.Success)
6054
+
6055
+	out, _ := dockerCmd(c, "run", "build2", "cmd.exe", "/s", "/c", "type", "c:\\bar")
6056
+	c.Assert(strings.TrimSpace(out), check.Equals, "abc")
6057
+	out, _ = dockerCmd(c, "run", "build2", "cmd.exe", "/s", "/c", "type", "c:\\foo")
6058
+	c.Assert(strings.TrimSpace(out), check.Equals, "def")
6059
+}
6060
+
6061
+func (s *DockerSuite) TestBuildCopyFromForbidWindowsSystemPaths(c *check.C) {
6062
+	testRequires(c, DaemonIsWindows)
6063
+	dockerfile := `
6064
+		FROM ` + testEnv.MinimalBaseImage() + `		
6065
+		FROM ` + testEnv.MinimalBaseImage() + `
6066
+		COPY --from=0 %s c:\\oscopy
6067
+		`
6068
+	exp := icmd.Expected{
6069
+		ExitCode: 1,
6070
+		Err:      "copy from c:\\ or c:\\windows is not allowed on windows",
6071
+	}
6072
+	buildImage("testforbidsystempaths1", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\"))).Assert(c, exp)
6073
+	buildImage("testforbidsystempaths2", build.WithDockerfile(fmt.Sprintf(dockerfile, "C:\\\\"))).Assert(c, exp)
6074
+	buildImage("testforbidsystempaths3", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\windows"))).Assert(c, exp)
6075
+	buildImage("testforbidsystempaths4", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:\\\\wInDows"))).Assert(c, exp)
6076
+}
6077
+
6078
+func (s *DockerSuite) TestBuildCopyFromForbidWindowsRelativePaths(c *check.C) {
6079
+	testRequires(c, DaemonIsWindows)
6080
+	dockerfile := `
6081
+		FROM ` + testEnv.MinimalBaseImage() + `		
6082
+		FROM ` + testEnv.MinimalBaseImage() + `
6083
+		COPY --from=0 %s c:\\oscopy
6084
+		`
6085
+	exp := icmd.Expected{
6086
+		ExitCode: 1,
6087
+		Err:      "copy from c:\\ or c:\\windows is not allowed on windows",
6088
+	}
6089
+	buildImage("testforbidsystempaths1", build.WithDockerfile(fmt.Sprintf(dockerfile, "c:"))).Assert(c, exp)
6090
+	buildImage("testforbidsystempaths2", build.WithDockerfile(fmt.Sprintf(dockerfile, "."))).Assert(c, exp)
6091
+	buildImage("testforbidsystempaths3", build.WithDockerfile(fmt.Sprintf(dockerfile, "..\\\\"))).Assert(c, exp)
6092
+	buildImage("testforbidsystempaths4", build.WithDockerfile(fmt.Sprintf(dockerfile, ".\\\\windows"))).Assert(c, exp)
6093
+	buildImage("testforbidsystempaths5", build.WithDockerfile(fmt.Sprintf(dockerfile, "\\\\windows"))).Assert(c, exp)
6094
+}
6095
+
6096
+func (s *DockerSuite) TestBuildCopyFromWindowsIsCaseInsensitive(c *check.C) {
6097
+	testRequires(c, DaemonIsWindows)
6098
+	dockerfile := `
6099
+		FROM ` + testEnv.MinimalBaseImage() + `
6100
+		COPY foo /	
6101
+		FROM ` + testEnv.MinimalBaseImage() + `
6102
+		COPY --from=0 c:\\fOo c:\\copied
6103
+		RUN type c:\\copied
6104
+		`
6105
+	ctx := fakeContext(c, dockerfile, map[string]string{
6106
+		"Dockerfile": dockerfile,
6107
+		"foo":        "hello world",
6108
+	})
6109
+	defer ctx.Close()
6110
+	exp := icmd.Expected{
6111
+		ExitCode: 0,
6112
+		Out:      "hello world",
6113
+	}
6114
+	result := buildImage("copyfrom-windows-insensitive", withExternalBuildContext(ctx))
6115
+	result.Assert(c, exp)
6116
+}
6117
+
6027 6118
 // TestBuildOpaqueDirectory tests that a build succeeds which
6028 6119
 // creates opaque directories.
6029 6120
 // See https://github.com/docker/docker/issues/25244