From 73d5baf585e3ef55864abeef43d45fe0b3a1c2bc Mon Sep 17 00:00:00 2001 From: Tibor Vass Date: Mon, 2 Feb 2015 16:17:12 -0500 Subject: [PATCH] builder: prevent Dockerfile to leave build context Signed-off-by: Tibor Vass --- api/client/commands.go | 47 +++++++------- builder/evaluator.go | 13 ++-- integration-cli/docker_api_containers_test.go | 43 +++++++++++-- integration-cli/docker_cli_build_test.go | 64 +++++++++++++++++++ 4 files changed, 132 insertions(+), 35 deletions(-) diff --git a/api/client/commands.go b/api/client/commands.go index 3cd110b4ec..d60c6d4827 100644 --- a/api/client/commands.go +++ b/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) - } - - // Now reset the dockerfileName to be relative to the build context - *dockerfileName = filename[len(absRoot):] } - if _, err = os.Stat(filename); os.IsNotExist(err) { - return fmt.Errorf("Can not locate Dockerfile: %s", origDockerfile) + origDockerfile := *dockerfileName // used for error msg + + if filename, err = filepath.Abs(filename); err != nil { + return err } - var includes []string = []string{"."} + + // 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.Lstat(filename); os.IsNotExist(err) { + return fmt.Errorf("Cannot locate Dockerfile: %s", origDockerfile) + } + var includes = []string{"."} excludes, err := utils.ReadDockerIgnore(path.Join(root, ".dockerignore")) if err != nil { diff --git a/builder/evaluator.go b/builder/evaluator.go index 0a6122cf21..b76c7f29bb 100644 --- a/builder/evaluator.go +++ b/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) } diff --git a/integration-cli/docker_api_containers_test.go b/integration-cli/docker_api_containers_test.go index eb7d27c95a..5dce388381 100644 --- a/integration-cli/docker_api_containers_test.go +++ b/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") +} diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index 2c04ba8f27..fad38317ef 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/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") +}