Merge pull request #61298 from brendandburns/kubectl

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Fix a bug where malformed paths don't get written to the destination dir

Fixes https://github.com/kubernetes/kubernetes/issues/61297

@liggitt @hansmi @cjcullen @jessfraz

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)))
 			}
 		}
 

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)
+		}
+	}
+}