artifact/template: prevent file sandbox escapes

Ensure that the client honors the client configuration for the
`template.disable_file_sandbox` field when validating the jobspec's
`template.source` parameter, and not just with consul-template's own `file`
function.

Prevent interpolated `template.source`, `template.destination`, and
`artifact.destination` fields from escaping file sandbox.

client/allocrunner/taskrunner/getter/getter.go

@@ -1,13 +1,14 @@
 package getter
 
 import (
+	"errors"
 	"fmt"
 	"net/url"
-	"path/filepath"
 	"strings"
 	"sync"
 
 	gg "github.com/hashicorp/go-getter"
+	"github.com/hashicorp/nomad/helper"
 	"github.com/hashicorp/nomad/nomad/structs"
 )
 
@@ -96,8 +97,12 @@ func GetArtifact(taskEnv EnvReplacer, artifact *structs.TaskArtifact, taskDir st
 		return newGetError(artifact.GetterSource, err, false)
 	}
 
-	// Download the artifact
-	dest := filepath.Join(taskDir, artifact.RelativeDest)
+	// Verify the destination is still in the task sandbox after interpolation
+	dest, err := helper.GetPathInSandbox(taskDir, artifact.RelativeDest)
+	if err != nil {
+		return newGetError(artifact.RelativeDest,
+			errors.New("artifact destination path escapes the alloc directory"), false)
+	}
 
 	// Convert from string getter mode to go-getter const
 	mode := gg.ClientModeAny

client/allocrunner/taskrunner/getter/getter_test.go

@@ -93,6 +93,36 @@ func TestGetArtifact_File_RelativeDest(t *testing.T) {
 	}
 }
 
+func TestGetArtifact_File_EscapeDest(t *testing.T) {
+	// Create the test server hosting the file to download
+	ts := httptest.NewServer(http.FileServer(http.Dir(filepath.Dir("./test-fixtures/"))))
+	defer ts.Close()
+
+	// Create a temp directory to download into
+	taskDir, err := ioutil.TempDir("", "nomad-test")
+	if err != nil {
+		t.Fatalf("failed to make temp directory: %v", err)
+	}
+	defer os.RemoveAll(taskDir)
+
+	// Create the artifact
+	file := "test.sh"
+	relative := "../../../../foo/"
+	artifact := &structs.TaskArtifact{
+		GetterSource: fmt.Sprintf("%s/%s", ts.URL, file),
+		GetterOptions: map[string]string{
+			"checksum": "md5:bce963762aa2dbfed13caf492a45fb72",
+		},
+		RelativeDest: relative,
+	}
+
+	// attempt to download the artifact
+	err = GetArtifact(taskEnv, artifact, taskDir)
+	if err == nil || !strings.Contains(err.Error(), "escapes") {
+		t.Fatalf("expected GetArtifact to disallow sandbox escape: %v", err)
+	}
+}
+
 func TestGetGetterUrl_Interpolation(t *testing.T) {
 	// Create the artifact
 	artifact := &structs.TaskArtifact{

client/allocrunner/taskrunner/template/template.go

@@ -28,10 +28,6 @@ const (
 	// consulTemplateSourceName is the source name when using the TaskHooks.
 	consulTemplateSourceName = "Template"
 
-	// hostSrcOption is the Client option that determines whether the template
-	// source may be from the host
-	hostSrcOption = "template.allow_host_source"
-
 	// missingDepEventLimit is the number of missing dependencies that will be
 	// logged before we switch to showing just the number of missing
 	// dependencies.
@@ -549,25 +545,27 @@ func maskProcessEnv(env map[string]string) map[string]string {
 // parseTemplateConfigs converts the tasks templates in the config into
 // consul-templates
 func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.TemplateConfig]*structs.Template, error) {
-	allowAbs := config.ClientConfig.ReadBoolDefault(hostSrcOption, true)
+	sandboxEnabled := !config.ClientConfig.TemplateConfig.DisableSandbox
 	taskEnv := config.EnvBuilder.Build()
 
 	ctmpls := make(map[*ctconf.TemplateConfig]*structs.Template, len(config.Templates))
 	for _, tmpl := range config.Templates {
 		var src, dest string
+		var err error
 		if tmpl.SourcePath != "" {
-			if filepath.IsAbs(tmpl.SourcePath) {
-				if !allowAbs {
-					return nil, fmt.Errorf("Specifying absolute template paths disallowed by client config: %q", tmpl.SourcePath)
-				}
-
-				src = tmpl.SourcePath
-			} else {
-				src = filepath.Join(config.TaskDir, taskEnv.ReplaceEnv(tmpl.SourcePath))
+			src = taskEnv.ReplaceEnv(tmpl.SourcePath)
+			src, err = helper.GetPathInSandbox(config.TaskDir, src)
+			if err != nil && sandboxEnabled {
+				return nil, fmt.Errorf("template source path escapes alloc directory")
 			}
 		}
+
 		if tmpl.DestPath != "" {
-			dest = filepath.Join(config.TaskDir, taskEnv.ReplaceEnv(tmpl.DestPath))
+			dest = taskEnv.ReplaceEnv(tmpl.DestPath)
+			dest, err = helper.GetPathInSandbox(config.TaskDir, dest)
+			if err != nil && sandboxEnabled {
+				return nil, fmt.Errorf("template destination path escapes alloc directory")
+			}
 		}
 
 		ct := ctconf.DefaultTemplateConfig()
@@ -577,7 +575,7 @@ func parseTemplateConfigs(config *TaskTemplateManagerConfig) (map[*ctconf.Templa
 		ct.LeftDelim = &tmpl.LeftDelim
 		ct.RightDelim = &tmpl.RightDelim
 		ct.FunctionDenylist = config.ClientConfig.TemplateConfig.FunctionDenylist
-		if !config.ClientConfig.TemplateConfig.DisableSandbox {
+		if sandboxEnabled {
 			ct.SandboxPath = &config.TaskDir
 		}
 

client/allocrunner/taskrunner/template/template_test.go

@@ -382,7 +382,11 @@ func TestTaskTemplateManager_HostPath(t *testing.T) {
 	}
 
 	harness := newTestHarness(t, []*structs.Template{template}, false, false)
-	harness.start(t)
+	harness.config.TemplateConfig.DisableSandbox = true
+	err = harness.startWithErr()
+	if err != nil {
+		t.Fatalf("couldn't setup initial harness: %v", err)
+	}
 	defer harness.stop()
 
 	// Wait for the unblock
@@ -405,12 +409,46 @@ func TestTaskTemplateManager_HostPath(t *testing.T) {
 
 	// Change the config to disallow host sources
 	harness = newTestHarness(t, []*structs.Template{template}, false, false)
-	harness.config.Options = map[string]string{
-		hostSrcOption: "false",
+	err = harness.startWithErr()
+	if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") {
+		t.Fatalf("Expected absolute template path disallowed for %q: %v",
+			template.SourcePath, err)
 	}
-	if err := harness.startWithErr(); err == nil || !strings.Contains(err.Error(), "absolute") {
-		t.Fatalf("Expected absolute template path disallowed: %v", err)
+
+	template.SourcePath = "../../../../../../" + file
+	harness = newTestHarness(t, []*structs.Template{template}, false, false)
+	err = harness.startWithErr()
+	if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") {
+		t.Fatalf("Expected directory traversal out of %q disallowed for %q: %v",
+			harness.taskDir, template.SourcePath, err)
 	}
+
+	// Build a new task environment
+	a := mock.Alloc()
+	task := a.Job.TaskGroups[0].Tasks[0]
+	task.Name = TestTaskName
+	task.Meta = map[string]string{"ESCAPE": "../"}
+
+	template.SourcePath = "${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}${NOMAD_META_ESCAPE}" + file
+	harness = newTestHarness(t, []*structs.Template{template}, false, false)
+	harness.envBuilder = taskenv.NewBuilder(harness.node, a, task, "global")
+	err = harness.startWithErr()
+	if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") {
+		t.Fatalf("Expected directory traversal out of %q via interpolation disallowed for %q: %v",
+			harness.taskDir, template.SourcePath, err)
+	}
+
+	// Test with desination too
+	template.SourcePath = f.Name()
+	template.DestPath = "../../../../../../" + file
+	harness = newTestHarness(t, []*structs.Template{template}, false, false)
+	harness.envBuilder = taskenv.NewBuilder(harness.node, a, task, "global")
+	err = harness.startWithErr()
+	if err == nil || !strings.Contains(err.Error(), "escapes alloc directory") {
+		t.Fatalf("Expected directory traversal out of %q via interpolation disallowed for %q: %v",
+			harness.taskDir, template.SourcePath, err)
+	}
+
 }
 
 func TestTaskTemplateManager_Unblock_Static(t *testing.T) {

helper/funcs.go

@@ -513,3 +513,22 @@ func CheckNamespaceScope(provided string, requested []string) []string {
 	}
 	return nil
 }
+
+// GetPathInSandbox returns a cleaned path inside the sandbox directory
+// (typically this will be the allocation directory). Relative paths will be
+// joined to the sandbox directory. Returns an error if the path escapes the
+// sandbox directory.
+func GetPathInSandbox(sandboxDir, path string) (string, error) {
+	if !filepath.IsAbs(path) {
+		path = filepath.Join(sandboxDir, path)
+	}
+	path = filepath.Clean(path)
+	rel, err := filepath.Rel(sandboxDir, path)
+	if err != nil {
+		return path, err
+	}
+	if strings.HasPrefix(rel, "..") {
+		return path, fmt.Errorf("%q escapes sandbox directory", path)
+	}
+	return path, nil
+}

helper/funcs_test.go

@@ -244,3 +244,72 @@ func TestCheckNamespaceScope(t *testing.T) {
 		})
 	}
 }
+
+func TestGetPathInSandbox(t *testing.T) {
+	cases := []struct {
+		name        string
+		path        string
+		dir         string
+		expected    string
+		expectedErr string
+	}{
+		{
+			name:     "ok absolute path inside sandbox",
+			path:     "/alloc/safe",
+			dir:      "/alloc",
+			expected: "/alloc/safe",
+		},
+		{
+			name:     "ok relative path inside sandbox",
+			path:     "./safe",
+			dir:      "/alloc",
+			expected: "/alloc/safe",
+		},
+		{
+			name:     "ok relative path traversal constrained to sandbox",
+			path:     "../../alloc/safe",
+			dir:      "/alloc",
+			expected: "/alloc/safe",
+		},
+		{
+			name:     "ok absolute path traversal constrained to sandbox",
+			path:     "/../alloc/safe",
+			dir:      "/alloc",
+			expected: "/alloc/safe",
+		},
+		{
+			name:        "fail absolute path outside sandbox",
+			path:        "/unsafe",
+			dir:         "/alloc",
+			expected:    "/unsafe",
+			expectedErr: "\"/unsafe\" escapes sandbox directory",
+		},
+		{
+			name:        "fail relative path traverses outside sandbox",
+			path:        "../../../unsafe",
+			dir:         "/alloc",
+			expected:    "/unsafe",
+			expectedErr: "\"/unsafe\" escapes sandbox directory",
+		},
+		{
+			name:        "fail absolute path tries to transverse outside sandbox",
+			path:        "/alloc/../unsafe",
+			dir:         "/alloc",
+			expected:    "/unsafe",
+			expectedErr: "\"/unsafe\" escapes sandbox directory",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			caseMsg := fmt.Sprintf("path: %v\ndir: %v", tc.path, tc.dir)
+			escapes, err := GetPathInSandbox(tc.dir, tc.path)
+			if tc.expectedErr != "" {
+				require.EqualError(t, err, tc.expectedErr, caseMsg)
+			} else {
+				require.NoError(t, err, caseMsg)
+			}
+			require.Equal(t, tc.expected, escapes, caseMsg)
+		})
+	}
+}