From ac3781f5323688c504ca968729d12cb1141ab730 Mon Sep 17 00:00:00 2001
From: Matt Butcher <matt.butcher@microsoft.com>
Date: Wed, 9 Jan 2019 17:26:11 -0700
Subject: [PATCH] fix: perform extra validation on paths in tar archives

Signed-off-by: Matt Butcher <matt.butcher@microsoft.com>
---
 pkg/chartutil/load.go      | 14 +++++++++
 pkg/chartutil/load_test.go | 61 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)

diff --git a/pkg/chartutil/load.go b/pkg/chartutil/load.go
index 9f1c80c85..0e077b1b8 100644
--- a/pkg/chartutil/load.go
+++ b/pkg/chartutil/load.go
@@ -25,6 +25,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"path"
 	"path/filepath"
 	"strings"
 
@@ -101,6 +102,19 @@ func LoadArchive(in io.Reader) (*chart.Chart, error) {
 		// Normalize the path to the / delimiter
 		n = strings.Replace(n, delimiter, "/", -1)
 
+		if path.IsAbs(n) {
+			return nil, errors.New("chart illegally contains absolute paths")
+		}
+
+		println("path", n)
+		n = path.Clean(n)
+		if n == "." {
+			return nil, errors.New("chart illegally contains empty path")
+		}
+		if strings.HasPrefix(n, "..") {
+			return nil, errors.New("chart illegally references parent directory")
+		}
+
 		if parts[0] == "Chart.yaml" {
 			return nil, errors.New("chart yaml not in base directory")
 		}
diff --git a/pkg/chartutil/load_test.go b/pkg/chartutil/load_test.go
index 5cb15fbdc..207aae641 100644
--- a/pkg/chartutil/load_test.go
+++ b/pkg/chartutil/load_test.go
@@ -17,8 +17,14 @@ limitations under the License.
 package chartutil
 
 import (
+	"archive/tar"
+	"compress/gzip"
+	"io/ioutil"
+	"os"
 	"path"
+	"path/filepath"
 	"testing"
+	"time"
 
 	"k8s.io/helm/pkg/proto/hapi/chart"
 )
@@ -43,6 +49,61 @@ func TestLoadFile(t *testing.T) {
 	verifyRequirements(t, c)
 }
 
+func TestLoadArchive_InvalidArchive(t *testing.T) {
+	tmpdir, err := ioutil.TempDir("", "helm-test-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(tmpdir)
+
+	writeTar := func(filename, internalPath string, body []byte) {
+		dest, err := os.Create(filename)
+		if err != nil {
+			t.Fatal(err)
+		}
+		zipper := gzip.NewWriter(dest)
+		tw := tar.NewWriter(zipper)
+
+		h := &tar.Header{
+			Name:    internalPath,
+			Mode:    0755,
+			Size:    int64(len(body)),
+			ModTime: time.Now(),
+		}
+		if err := tw.WriteHeader(h); err != nil {
+			t.Fatal(err)
+		}
+		if _, err := tw.Write(body); err != nil {
+			t.Fatal(err)
+		}
+		tw.Close()
+		zipper.Close()
+		dest.Close()
+	}
+
+	for _, tt := range []struct {
+		chartname   string
+		internal    string
+		expectError string
+	}{
+		{"illegal-dots.tgz", "../../malformed-helm-test", "chart illegally references parent directory"},
+		{"illegal-dots2.tgz", "foo/../../malformed-helm-test", "chart illegally references parent directory"},
+		{"illegal-name.tgz", ".", "chart illegally contains empty path"},
+		{"illegal-name.tgz", " ", "chart illegally contains empty path"},
+		{"illegal-name.tgz", "       ", "chart illegally contains empty path"},
+	} {
+		illegalChart := filepath.Join(tmpdir, tt.chartname)
+		writeTar(illegalChart, tt.internal, []byte("junk"))
+		_, err = Load(illegalChart)
+		if err == nil {
+			t.Fatal("expected error when unpacking illegal files")
+		}
+		if err.Error() != tt.expectError {
+			t.Errorf("Expected %q, got %q", tt.expectError, err.Error())
+		}
+	}
+}
+
 func TestLoadFiles(t *testing.T) {
 	goodFiles := []*BufferedFile{
 		{