From d196afabc440a0620c938285cefa366cd0cfe096 Mon Sep 17 00:00:00 2001
From: Brendan Burns <bburns@microsoft.com>
Date: Fri, 16 Mar 2018 12:19:31 -0700
Subject: [PATCH] Fix a bug where malformed paths don't get written to the
 destination dir.

---
 pkg/kubectl/cmd/cp.go      | 10 ++++-
 pkg/kubectl/cmd/cp_test.go | 75 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 2 deletions(-)

diff --git a/pkg/kubectl/cmd/cp.go b/pkg/kubectl/cmd/cp.go
index 7a39714dbd..602be6d0ab 100644
--- a/pkg/kubectl/cmd/cp.go
+++ b/pkg/kubectl/cmd/cp.go
@@ -312,6 +312,12 @@ func recursiveTar(srcBase, srcFile, destBase, destFile string, tw *tar.Writer) e
 	return nil
 }
 
+// clean prevents path traversals by stripping them out.
+// This is adapted from https://golang.org/src/net/http/fs.go#L74
+func clean(fileName string) string {
+	return path.Clean(string(os.PathSeparator) + fileName)
+}
+
 func untarAll(reader io.Reader, destFile, prefix string) error {
 	entrySeq := -1
 
@@ -327,7 +333,7 @@ func untarAll(reader io.Reader, destFile, prefix string) error {
 		}
 		entrySeq++
 		mode := header.FileInfo().Mode()
-		outFileName := path.Join(destFile, header.Name[len(prefix):])
+		outFileName := path.Join(destFile, clean(header.Name[len(prefix):]))
 		baseName := path.Dir(outFileName)
 		if err := os.MkdirAll(baseName, 0755); err != nil {
 			return err
@@ -346,7 +352,7 @@ func untarAll(reader io.Reader, destFile, prefix string) error {
 				return err
 			}
 			if exists {
-				outFileName = filepath.Join(outFileName, path.Base(header.Name))
+				outFileName = filepath.Join(outFileName, path.Base(clean(header.Name)))
 			}
 		}
 
diff --git a/pkg/kubectl/cmd/cp_test.go b/pkg/kubectl/cmd/cp_test.go
index 8236fef7a3..1c04812959 100644
--- a/pkg/kubectl/cmd/cp_test.go
+++ b/pkg/kubectl/cmd/cp_test.go
@@ -422,3 +422,78 @@ func TestTarDestinationName(t *testing.T) {
 		}
 	}
 }
+
+func TestBadTar(t *testing.T) {
+	dir, err := ioutil.TempDir(os.TempDir(), "dest")
+	if err != nil {
+		t.Errorf("unexpected error: %v ", err)
+		t.FailNow()
+	}
+	defer os.RemoveAll(dir)
+
+	// More or less cribbed from https://golang.org/pkg/archive/tar/#example__minimal
+	var buf bytes.Buffer
+	tw := tar.NewWriter(&buf)
+	var files = []struct {
+		name string
+		body string
+	}{
+		{"/prefix/../../../tmp/foo", "Up to temp"},
+		{"/prefix/foo/bar/../../home/bburns/names.txt", "Down and back"},
+	}
+	for _, file := range files {
+		hdr := &tar.Header{
+			Name: file.name,
+			Mode: 0600,
+			Size: int64(len(file.body)),
+		}
+		if err := tw.WriteHeader(hdr); err != nil {
+			t.Errorf("unexpected error: %v ", err)
+			t.FailNow()
+		}
+		if _, err := tw.Write([]byte(file.body)); err != nil {
+			t.Errorf("unexpected error: %v ", err)
+			t.FailNow()
+		}
+	}
+	if err := tw.Close(); err != nil {
+		t.Errorf("unexpected error: %v ", err)
+		t.FailNow()
+	}
+
+	if err := untarAll(&buf, dir, "/prefix"); err != nil {
+		t.Errorf("unexpected error: %v ", err)
+		t.FailNow()
+	}
+
+	for _, file := range files {
+		_, err := os.Stat(path.Join(dir, path.Clean(file.name[len("/prefix"):])))
+		if err != nil {
+			t.Errorf("Error finding file: %v", err)
+		}
+	}
+
+}
+
+func TestClean(t *testing.T) {
+	tests := []struct {
+		input   string
+		cleaned string
+	}{
+		{
+			"../../../tmp/foo",
+			"/tmp/foo",
+		},
+		{
+			"/../../../tmp/foo",
+			"/tmp/foo",
+		},
+	}
+
+	for _, test := range tests {
+		out := clean(test.input)
+		if out != test.cleaned {
+			t.Errorf("Expected: %s, saw %s", test.cleaned, out)
+		}
+	}
+}