builder: prevent Dockerfile to leave build context

Signed-off-by: Tibor Vass <teabee89@gmail.com>

api/client/commands.go

@@ -39,6 +39,7 @@ import (
 	"github.com/docker/docker/pkg/parsers/filters"
 	"github.com/docker/docker/pkg/promise"
 	"github.com/docker/docker/pkg/signal"
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/term"
 	"github.com/docker/docker/pkg/timeutils"
 	"github.com/docker/docker/pkg/units"
@@ -147,36 +148,36 @@ func (cli *DockerCli) CmdBuild(args ...string) error {
 			return err
 		}
 
-		var filename string       // path to Dockerfile
-		var origDockerfile string // used for error msg
+		filename := *dockerfileName // path to Dockerfile
 
 		if *dockerfileName == "" {
 			// No -f/--file was specified so use the default
-			origDockerfile = api.DefaultDockerfileName
-			*dockerfileName = origDockerfile
+			*dockerfileName = api.DefaultDockerfileName
 			filename = path.Join(absRoot, *dockerfileName)
-		} else {
-			origDockerfile = *dockerfileName
-			if filename, err = filepath.Abs(*dockerfileName); err != nil {
-				return err
-			}
+		}
 
-			// Verify that 'filename' is within the build context
-			if !strings.HasSuffix(absRoot, string(os.PathSeparator)) {
-				absRoot += string(os.PathSeparator)
-			}
-			if !strings.HasPrefix(filename, absRoot) {
-				return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", *dockerfileName, root)
-			}
+		origDockerfile := *dockerfileName // used for error msg
+
+		if filename, err = filepath.Abs(filename); err != nil {
+			return err
+		}
 
-			// Now reset the dockerfileName to be relative to the build context
-			*dockerfileName = filename[len(absRoot):]
+		// Verify that 'filename' is within the build context
+		filename, err = symlink.FollowSymlinkInScope(filename, absRoot)
+		if err != nil {
+			return fmt.Errorf("The Dockerfile (%s) must be within the build context (%s)", origDockerfile, root)
+		}
+
+		// Now reset the dockerfileName to be relative to the build context
+		*dockerfileName, err = filepath.Rel(filename, absRoot)
+		if err != nil {
+			return err
 		}
 
-		if _, err = os.Stat(filename); os.IsNotExist(err) {
-			return fmt.Errorf("Can not locate Dockerfile: %s", origDockerfile)
+		if _, err = os.Lstat(filename); os.IsNotExist(err) {
+			return fmt.Errorf("Cannot locate Dockerfile: %s", origDockerfile)
 		}
-		var includes []string = []string{"."}
+		var includes = []string{"."}
 
 		excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore"))
 		if err != nil {

builder/evaluator.go

@@ -32,6 +32,7 @@ import (
 	"github.com/docker/docker/daemon"
 	"github.com/docker/docker/engine"
 	"github.com/docker/docker/pkg/fileutils"
+	"github.com/docker/docker/pkg/symlink"
 	"github.com/docker/docker/pkg/tarsum"
 	"github.com/docker/docker/registry"
 	"github.com/docker/docker/runconfig"
@@ -170,16 +171,12 @@ func (b *Builder) Run(context io.Reader) (string, error) {
 // Reads a Dockerfile from the current context. It assumes that the
 // 'filename' is a relative path from the root of the context
 func (b *Builder) readDockerfile(origFile string) error {
-	filename := filepath.Join(b.contextPath, origFile)
-
-	tmpDockerPath := filepath.Dir(filename) + string(os.PathSeparator)
-	tmpContextPath := filepath.Clean(b.contextPath) + string(os.PathSeparator)
-
-	if !strings.HasPrefix(tmpDockerPath, tmpContextPath) {
-		return fmt.Errorf("Dockerfile (%s) must be within the build context", origFile)
+	filename, err := symlink.FollowSymlinkInScope(filepath.Join(b.contextPath, origFile), b.contextPath)
+	if err != nil {
+		return fmt.Errorf("The Dockerfile (%s) must be within the build context", origFile)
 	}
 
-	fi, err := os.Stat(filename)
+	fi, err := os.Lstat(filename)
 	if os.IsNotExist(err) {
 		return fmt.Errorf("Cannot locate specified Dockerfile: %s", origFile)
 	}

integration-cli/docker_api_containers_test.go

@@ -307,13 +307,14 @@ func TestBuildApiDockerfilePath(t *testing.T) {
 	tw := tar.NewWriter(buffer)
 	defer tw.Close()
 
+	dockerfile := []byte("FROM busybox")
 	if err := tw.WriteHeader(&tar.Header{
 		Name: "Dockerfile",
-		Size: 11,
+		Size: int64(len(dockerfile)),
 	}); err != nil {
 		t.Fatalf("failed to write tar file header: %v", err)
 	}
-	if _, err := tw.Write([]byte("FROM ubuntu")); err != nil {
+	if _, err := tw.Write(dockerfile); err != nil {
 		t.Fatalf("failed to write tar file content: %v", err)
 	}
 	if err := tw.Close(); err != nil {
@@ -322,12 +323,46 @@ func TestBuildApiDockerfilePath(t *testing.T) {
 
 	out, err := sockRequestRaw("POST", "/build?dockerfile=../Dockerfile", buffer, "application/x-tar")
 	if err == nil {
-		t.Fatalf("Build was supposed to fail")
+		t.Fatalf("Build was supposed to fail: %s", out)
 	}
 
 	if !strings.Contains(string(out), "must be within the build context") {
-		t.Fatalf("Didn't complain about leaving build context")
+		t.Fatalf("Didn't complain about leaving build context: %s", out)
 	}
 
 	logDone("container REST API - check build w/bad Dockerfile path")
 }
+
+func TestBuildApiDockerfileSymlink(t *testing.T) {
+	// Test to make sure we stop people from trying to leave the
+	// build context when specifying a symlink as the path to the dockerfile
+	buffer := new(bytes.Buffer)
+	tw := tar.NewWriter(buffer)
+	defer tw.Close()
+
+	if err := tw.WriteHeader(&tar.Header{
+		Name:     "Dockerfile",
+		Typeflag: tar.TypeSymlink,
+		Linkname: "/etc/passwd",
+	}); err != nil {
+		t.Fatalf("failed to write tar file header: %v", err)
+	}
+	if err := tw.Close(); err != nil {
+		t.Fatalf("failed to close tar archive: %v", err)
+	}
+
+	out, err := sockRequestRaw("POST", "/build", buffer, "application/x-tar")
+	if err == nil {
+		t.Fatalf("Build was supposed to fail: %s", out)
+	}
+
+	// The reason the error is "Cannot locate specified Dockerfile" is because
+	// in the builder, the symlink is resolved within the context, therefore
+	// Dockerfile -> /etc/passwd becomes etc/passwd from the context which is
+	// a nonexistent file.
+	if !strings.Contains(string(out), "Cannot locate specified Dockerfile: Dockerfile") {
+		t.Fatalf("Didn't complain about leaving build context: %s", out)
+	}
+
+	logDone("container REST API - check build w/bad Dockerfile symlink path")
+}

integration-cli/docker_cli_build_test.go

@@ -4630,3 +4630,67 @@ func TestBuildFromOfficialNames(t *testing.T) {
 	}
 	logDone("build - from official names")
 }
+
+func TestBuildDockerfileOutsideContext(t *testing.T) {
+	name := "testbuilddockerfileoutsidecontext"
+	tmpdir, err := ioutil.TempDir("", name)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpdir)
+	ctx := filepath.Join(tmpdir, "context")
+	if err := os.MkdirAll(ctx, 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(ctx, "Dockerfile"), []byte("FROM busybox"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	wd, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Chdir(wd)
+	if err := os.Chdir(ctx); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(filepath.Join(tmpdir, "outsideDockerfile"), []byte("FROM busbox"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink("../outsideDockerfile", filepath.Join(ctx, "dockerfile1")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Symlink(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile2")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Link("../outsideDockerfile", filepath.Join(ctx, "dockerfile3")); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Link(filepath.Join(tmpdir, "outsideDockerfile"), filepath.Join(ctx, "dockerfile4")); err != nil {
+		t.Fatal(err)
+	}
+	for _, dockerfilePath := range []string{
+		"../outsideDockerfile",
+		filepath.Join(ctx, "dockerfile1"),
+		filepath.Join(ctx, "dockerfile2"),
+		filepath.Join(ctx, "dockerfile3"),
+		filepath.Join(ctx, "dockerfile4"),
+	} {
+		out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", dockerfilePath, "."))
+		if err == nil {
+			t.Fatalf("Expected error with %s. Out: %s", dockerfilePath, out)
+		}
+		deleteImages(name)
+	}
+
+	os.Chdir(tmpdir)
+
+	// Path to Dockerfile should be resolved relative to working directory, not relative to context.
+	// There is a Dockerfile in the context, but since there is no Dockerfile in the current directory, the following should fail
+	out, _, err := runCommandWithOutput(exec.Command(dockerBinary, "build", "-t", name, "--no-cache", "-f", "Dockerfile", ctx))
+	if err == nil {
+		t.Fatalf("Expected error. Out: %s", out)
+	}
+	deleteImages(name)
+
+	logDone("build - Dockerfile outside context")
+}