Merge pull request #842 from laverya/block-writing-to-parent-dir

Prevent assets from writing files outside of the dir ship is run in

pkg/lifecycle/render/docker/step.go

@@ -14,6 +14,8 @@ import (
 	"github.com/replicatedhq/ship/pkg/images"
 	"github.com/replicatedhq/ship/pkg/lifecycle/render/root"
 	"github.com/replicatedhq/ship/pkg/templates"
+	"github.com/replicatedhq/ship/pkg/util"
+
 	"github.com/spf13/viper"
 )
 
@@ -96,6 +98,11 @@ func (p *DefaultStep) Execute(
 		}
 		destIsDockerURL := destinationURL.Scheme == "docker"
 		if !destIsDockerURL {
+			err = util.IsLegalPath(dest)
+			if err != nil {
+				return errors.Wrap(err, "find docker image dest")
+			}
+
 			basePath := filepath.Dir(dest)
 			debug.Log("event", "mkdirall.attempt", "dest", dest, "basePath", basePath)
 			if err := rootFs.MkdirAll(basePath, 0755); err != nil {

pkg/lifecycle/render/dockerlayer/layer.go

@@ -12,6 +12,8 @@ import (
 	"github.com/replicatedhq/ship/pkg/api"
 	"github.com/replicatedhq/ship/pkg/lifecycle/render/docker"
 	"github.com/replicatedhq/ship/pkg/lifecycle/render/root"
+	"github.com/replicatedhq/ship/pkg/util"
+
 	"github.com/spf13/afero"
 	"github.com/spf13/viper"
 )
@@ -66,6 +68,11 @@ func (u *Unpacker) Execute(
 			return errors.Wrap(err, "resolve unpack paths")
 		}
 
+		err = util.IsLegalPath(basePath)
+		if err != nil {
+			return errors.Wrap(err, "write docker layer")
+		}
+
 		debug.Log(
 			"event", "execute",
 			"savePath", savePath,

pkg/lifecycle/render/github/render.go

@@ -21,6 +21,7 @@ import (
 	"github.com/replicatedhq/ship/pkg/specs/gogetter"
 	"github.com/replicatedhq/ship/pkg/state"
 	"github.com/replicatedhq/ship/pkg/templates"
+	"github.com/replicatedhq/ship/pkg/util"
 
 	"github.com/spf13/afero"
 	"github.com/spf13/viper"
@@ -263,7 +264,14 @@ func getDestPath(githubPath string, asset api.GitHubAsset, builder *templates.Bu
 		}
 	}
 
-	return filepath.Join(destDir, githubPath), nil
+	combinedPath := filepath.Join(destDir, githubPath)
+
+	err = util.IsLegalPath(combinedPath)
+	if err != nil {
+		return "", errors.Wrap(err, "write github asset")
+	}
+
+	return combinedPath, nil
 }
 
 func (r *LocalRenderer) getDestPathNoProxy(asset api.GitHubAsset, builder *templates.Builder, renderRoot string) (string, error) {

pkg/lifecycle/render/github/render_test.go

@@ -206,6 +206,36 @@ func Test_getDestPath(t *testing.T) {
 			want:    "",
 			wantErr: true,
 		},
+		{
+			name: "file in root",
+			args: args{
+				githubPath: "subdir/README.md",
+				asset: api.GitHubAsset{
+					Path:      "",
+					StripPath: "",
+					AssetShared: api.AssetShared{
+						Dest: "/bin/runc",
+					},
+				},
+			},
+			want:    "",
+			wantErr: true,
+		},
+		{
+			name: "file in parent dir",
+			args: args{
+				githubPath: "subdir/README.md",
+				asset: api.GitHubAsset{
+					Path:      "abc/",
+					StripPath: "",
+					AssetShared: api.AssetShared{
+						Dest: "../../../bin/runc",
+					},
+				},
+			},
+			want:    "",
+			wantErr: true,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/lifecycle/render/helm/template.go

@@ -324,6 +324,11 @@ func (f *LocalTemplater) cleanUpAndOutputRenderedFiles(
 	tempRenderedChartTemplatesDir := path.Join(tempRenderedChartDir, "templates")
 	tempRenderedSubChartsDir := path.Join(tempRenderedChartDir, subChartsDirName)
 
+	err := util.IsLegalPath(asset.Dest)
+	if err != nil {
+		return errors.Wrap(err, "write helm asset")
+	}
+
 	if f.Viper.GetBool("rm-asset-dest") {
 		debug.Log("event", "baseDir.rm", "path", asset.Dest)
 		if err := f.FS.RemoveAll(asset.Dest); err != nil {

pkg/lifecycle/render/inline/render.go

@@ -12,6 +12,8 @@ import (
 	"github.com/replicatedhq/ship/pkg/api"
 	"github.com/replicatedhq/ship/pkg/lifecycle/render/root"
 	"github.com/replicatedhq/ship/pkg/templates"
+	"github.com/replicatedhq/ship/pkg/util"
+
 	"github.com/spf13/viper"
 )
 
@@ -65,29 +67,51 @@ func (r *LocalRenderer) Execute(
 			return errors.Wrap(err, "init builder")
 		}
 
-		built, err := builder.String(asset.Contents)
+		builtAsset, err := templateInline(builder, asset)
 		if err != nil {
 			return errors.Wrap(err, "building contents")
 		}
 
+		err = util.IsLegalPath(builtAsset.Dest)
+		if err != nil {
+			return errors.Wrap(err, "write inline asset")
+		}
+
 		basePath := filepath.Dir(asset.Dest)
-		debug.Log("event", "mkdirall.attempt", "dest", asset.Dest, "basePath", basePath)
+		debug.Log("event", "mkdirall.attempt", "dest", builtAsset.Dest, "basePath", basePath)
 		if err := rootFs.MkdirAll(basePath, 0755); err != nil {
-			debug.Log("event", "mkdirall.fail", "err", err, "dest", asset.Dest, "basePath", basePath)
-			return errors.Wrapf(err, "write directory to %s", asset.Dest)
+			debug.Log("event", "mkdirall.fail", "err", err, "dest", builtAsset.Dest, "basePath", basePath)
+			return errors.Wrapf(err, "write directory to %s", builtAsset.Dest)
 		}
 
 		mode := os.FileMode(0644)
-		if asset.Mode != os.FileMode(0) {
+		if builtAsset.Mode != os.FileMode(0) {
 			debug.Log("event", "applying override permissions")
-			mode = asset.Mode
+			mode = builtAsset.Mode
 		}
 
-		if err := rootFs.WriteFile(asset.Dest, []byte(built), mode); err != nil {
+		if err := rootFs.WriteFile(builtAsset.Dest, []byte(builtAsset.Contents), mode); err != nil {
 			debug.Log("event", "execute.fail", "err", err)
-			return errors.Wrapf(err, "Write inline asset to %s", asset.Dest)
+			return errors.Wrapf(err, "Write inline asset to %s", builtAsset.Dest)
 		}
 		return nil
 
 	}
 }
+
+func templateInline(builder *templates.Builder, asset api.InlineAsset) (api.InlineAsset, error) {
+	builtAsset := asset
+	var err error
+
+	builtAsset.Contents, err = builder.String(asset.Contents)
+	if err != nil {
+		return builtAsset, errors.Wrap(err, "building contents")
+	}
+
+	builtAsset.Dest, err = builder.String(asset.Dest)
+	if err != nil {
+		return builtAsset, errors.Wrap(err, "building dest")
+	}
+
+	return builtAsset, nil
+}

pkg/lifecycle/render/inline/render_test.go

@@ -23,6 +23,7 @@ func TestInlineRender(t *testing.T) {
 		templateContext map[string]interface{}
 		configGroups    []libyaml.ConfigGroup
 		expect          map[string]interface{}
+		expectErr       bool
 	}{
 		{
 			name: "happy path",
@@ -40,6 +41,52 @@ func TestInlineRender(t *testing.T) {
 			templateContext: map[string]interface{}{},
 			configGroups:    []libyaml.ConfigGroup{},
 		},
+		{
+			name: "templated dest path",
+			asset: api.InlineAsset{
+				Contents: "hello!",
+				AssetShared: api.AssetShared{
+					Dest: "{{repl if true}}foo.txt{{repl else}}notfoo.txt{{repl end}}",
+				},
+			},
+			expect: map[string]interface{}{
+				"foo.txt": "hello!",
+			},
+
+			meta:            api.ReleaseMetadata{},
+			templateContext: map[string]interface{}{},
+			configGroups:    []libyaml.ConfigGroup{},
+		},
+		{
+			name: "absolute dest path",
+			asset: api.InlineAsset{
+				Contents: "hello!",
+				AssetShared: api.AssetShared{
+					Dest: "/bin/runc",
+				},
+			},
+			expect: map[string]interface{}{},
+
+			meta:            api.ReleaseMetadata{},
+			templateContext: map[string]interface{}{},
+			configGroups:    []libyaml.ConfigGroup{},
+			expectErr:       true,
+		},
+		{
+			name: "parent dir dest path",
+			asset: api.InlineAsset{
+				Contents: "hello!",
+				AssetShared: api.AssetShared{
+					Dest: "../../../bin/runc",
+				},
+			},
+			expect: map[string]interface{}{},
+
+			meta:            api.ReleaseMetadata{},
+			templateContext: map[string]interface{}{},
+			configGroups:    []libyaml.ConfigGroup{},
+			expectErr:       true,
+		},
 	}
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
@@ -65,7 +112,11 @@ func TestInlineRender(t *testing.T) {
 				test.templateContext,
 				test.configGroups,
 			)(context.Background())
-			req.NoError(err)
+			if !test.expectErr {
+				req.NoError(err)
+			} else {
+				req.Error(err)
+			}
 
 			for filename, expectContents := range test.expect {
 				contents, err := rootFs.ReadFile(filename)

pkg/lifecycle/render/local/render.go

@@ -9,6 +9,8 @@ import (
 	"github.com/pkg/errors"
 	"github.com/replicatedhq/libyaml"
 	"github.com/replicatedhq/ship/pkg/api"
+	"github.com/replicatedhq/ship/pkg/util"
+
 	"github.com/spf13/afero"
 )
 
@@ -47,6 +49,16 @@ func (r *LocalRenderer) Execute(
 	return func(ctx context.Context) error {
 		debug := level.Debug(log.With(r.Logger, "step.type", "render", "render.phase", "execute", "asset.type", "local"))
 
+		err := util.IsLegalPath(asset.Dest)
+		if err != nil {
+			return errors.Wrap(err, "local asset dest")
+		}
+
+		err = util.IsLegalPath(asset.Path)
+		if err != nil {
+			return errors.Wrap(err, "local asset path")
+		}
+
 		if err := r.Fs.MkdirAll(filepath.Dir(asset.Dest), 0777); err != nil {
 			return errors.Wrapf(err, "mkdir %s", asset.Dest)
 		}

pkg/lifecycle/render/web/step.go

@@ -14,6 +14,8 @@ import (
 	"github.com/replicatedhq/ship/pkg/api"
 	"github.com/replicatedhq/ship/pkg/lifecycle/render/root"
 	"github.com/replicatedhq/ship/pkg/templates"
+	"github.com/replicatedhq/ship/pkg/util"
+
 	"github.com/spf13/afero"
 	"github.com/spf13/viper"
 )
@@ -82,6 +84,11 @@ func (p *DefaultStep) Execute(
 			return errors.Wrapf(err, "Build web asset")
 		}
 
+		err = util.IsLegalPath(built.Dest)
+		if err != nil {
+			return errors.Wrap(err, "write web asset")
+		}
+
 		basePath := filepath.Dir(asset.Dest)
 		debug.Log("event", "mkdirall.attempt", "root", rootFs.RootPath, "dest", built.Dest, "basePath", basePath)
 		if err := rootFs.MkdirAll(basePath, 0755); err != nil {

pkg/lifecycle/render/web/step_test.go

@@ -200,6 +200,36 @@ func TestWebStep(t *testing.T) {
 			ExpectFiles: map[string]interface{}{},
 			ExpectedErr: errors.New("Get web asset from http://foo.bar: received response with status 500"),
 		},
+		{
+			Name: "illegal dest path",
+			Asset: api.WebAsset{
+				AssetShared: api.AssetShared{
+					Dest: "/bin/runc",
+				},
+				URL: "http://foo.bar",
+			},
+			RegisterResponders: func() {
+				httpmock.RegisterResponder("GET", "http://foo.bar",
+					httpmock.NewStringResponder(200, "hi from foo.bar"))
+			},
+			ExpectFiles: map[string]interface{}{},
+			ExpectedErr: errors.Wrap(errors.New("cannot write to an absolute path: /bin/runc"), "write web asset"),
+		},
+		{
+			Name: "illegal dest path",
+			Asset: api.WebAsset{
+				AssetShared: api.AssetShared{
+					Dest: "../../../bin/runc",
+				},
+				URL: "http://foo.bar",
+			},
+			RegisterResponders: func() {
+				httpmock.RegisterResponder("GET", "http://foo.bar",
+					httpmock.NewStringResponder(200, "hi from foo.bar"))
+			},
+			ExpectFiles: map[string]interface{}{},
+			ExpectedErr: errors.Wrap(errors.New("cannot write to a path that is a parent of the working dir: ../../../bin/runc"), "write web asset"),
+		},
 	}
 
 	for _, test := range tests {

pkg/util/legal_path.go

@@ -0,0 +1,40 @@
+package util
+
+import (
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/pkg/errors"
+)
+
+// IsLegalPath checks if the provided path is a relative path within the current working directory or within the os tempdir.
+// If it is not, it returns an error.
+func IsLegalPath(path string) error {
+
+	if filepath.IsAbs(path) {
+		relAbsPath, err := filepath.Rel(os.TempDir(), path)
+		if err != nil {
+			return fmt.Errorf("cannot write to an absolute path: %s, got error finding relative path from tempdir: %s", path, err.Error())
+		}
+
+		// subdirectories of the os tempdir are fine
+		if !strings.Contains(relAbsPath, "..") {
+			return nil
+		}
+
+		return fmt.Errorf("cannot write to an absolute path: %s", path)
+	}
+
+	relPath, err := filepath.Rel(".", path)
+	if err != nil {
+		return errors.Wrap(err, "find relative path to dest")
+	}
+
+	if strings.Contains(relPath, "..") {
+		return fmt.Errorf("cannot write to a path that is a parent of the working dir: %s", relPath)
+	}
+
+	return nil
+}