fix: Chart dot-name path bug

Signed-off-by: George Jenkins <gvjenkins@gmail.com>

pkg/chart/metadata.go

@@ -112,6 +112,9 @@ func (md *Metadata) Validate() error {
 		return ValidationError("chart.metadata.name is required")
 	}
 
+	if md.Name == "." || md.Name == ".." {
+		return ValidationErrorf("chart.metadata.name %q is not allowed", md.Name)
+	}
 	if md.Name != filepath.Base(md.Name) {
 		return ValidationErrorf("chart.metadata.name %q is invalid", md.Name)
 	}

pkg/chart/metadata_test.go

@@ -40,6 +40,16 @@ func TestValidate(t *testing.T) {
 			&Metadata{APIVersion: "v2", Version: "1.0"},
 			ValidationError("chart.metadata.name is required"),
 		},
+		{
+			"chart with dot name",
+			&Metadata{Name: ".", APIVersion: "v2", Version: "1.0"},
+			ValidationError("chart.metadata.name \".\" is not allowed"),
+		},
+		{
+			"chart with dotdot name",
+			&Metadata{Name: "..", APIVersion: "v2", Version: "1.0"},
+			ValidationError("chart.metadata.name \"..\" is not allowed"),
+		},
 		{
 			"chart without name",
 			&Metadata{Name: "../../test", APIVersion: "v2", Version: "1.0"},

pkg/chartutil/expand.go

@@ -17,6 +17,7 @@ limitations under the License.
 package chartutil
 
 import (
+	"fmt"
 	"io"
 	"os"
 	"path/filepath"
@@ -51,6 +52,17 @@ func Expand(dir string, r io.Reader) error {
 		return errors.New("chart name not specified")
 	}
 
+	// Reject chart names that are POSIX path dot-segments or dot-dot segments or contain path separators.
+	// A dot-segment name (e.g. ".") causes SecureJoin to resolve to the root
+	// directory and extraction then to write files directly into that extraction root
+	// instead of a per-chart subdirectory.
+	if chartName == "." || chartName == ".." {
+		return fmt.Errorf("chart name %q is not allowed", chartName)
+	}
+	if chartName != filepath.Base(chartName) {
+		return fmt.Errorf("chart name %q must not contain path separators", chartName)
+	}
+
 	// Find the base directory
 	// The directory needs to be cleaned prior to passing to SecureJoin or the location may end up
 	// being wrong or returning an error. This was introduced in v0.4.0.
@@ -60,6 +72,12 @@ func Expand(dir string, r io.Reader) error {
 		return err
 	}
 
+	// Defense-in-depth: the chart directory must be a subdirectory of dir,
+	// never dir itself.
+	if chartdir == dir {
+		return fmt.Errorf("chart name %q resolves to the extraction root", chartName)
+	}
+
 	// Copy all files verbatim. We don't parse these files because parsing can remove
 	// comments.
 	for _, file := range files {

pkg/chartutil/expand_test.go

@@ -17,11 +17,73 @@ limitations under the License.
 package chartutil
 
 import (
+	"archive/tar"
+	"bytes"
+	"compress/gzip"
+	"io/fs"
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+// makeTestChartArchive builds a gzipped tar archive from the given sourceDir directory, file entries are prefixed with the given chartName
+func makeTestChartArchive(t *testing.T, chartName, sourceDir string) *bytes.Buffer {
+	t.Helper()
+
+	var result bytes.Buffer
+	gw := gzip.NewWriter(&result)
+	tw := tar.NewWriter(gw)
+
+	dir := os.DirFS(sourceDir)
+
+	writeFile := func(relPath string) {
+		t.Helper()
+		f, err := dir.Open(relPath)
+		require.NoError(t, err)
+
+		fStat, err := f.Stat()
+		require.NoError(t, err)
+
+		err = tw.WriteHeader(&tar.Header{
+			Name: filepath.Join(chartName, relPath),
+			Mode: int64(fStat.Mode()),
+			Size: fStat.Size(),
+		})
+		require.NoError(t, err)
+
+		data, err := fs.ReadFile(dir, relPath)
+		require.NoError(t, err)
+		tw.Write(data)
+	}
+
+	err := fs.WalkDir(dir, ".", func(path string, d os.DirEntry, walkErr error) error {
+		if walkErr != nil {
+			return walkErr
+		}
+
+		if d.IsDir() {
+			return nil
+		}
+
+		writeFile(path)
+
+		return nil
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	err = tw.Close()
+	require.NoError(t, err)
+	err = gw.Close()
+	require.NoError(t, err)
+
+	return &result
+}
+
 func TestExpand(t *testing.T) {
 	dest := t.TempDir()
 
@@ -75,6 +137,28 @@ func TestExpand(t *testing.T) {
 	}
 }
 
+func TestExpandError(t *testing.T) {
+	tests := map[string]struct {
+		chartName string
+		chartDir  string
+		wantErr   string
+	}{
+		"dot name":      {"dotname", "testdata/dotname", "not allowed"},
+		"dotdot name":   {"dotdotname", "testdata/dotdotname", "not allowed"},
+		"slash in name": {"slashinname", "testdata/slashinname", "must not contain path separators"},
+	}
+
+	for name, tt := range tests {
+		t.Run(name, func(t *testing.T) {
+			archive := makeTestChartArchive(t, tt.chartName, tt.chartDir)
+
+			dest := t.TempDir()
+			err := Expand(dest, archive)
+			assert.ErrorContains(t, err, tt.wantErr)
+		})
+	}
+}
+
 func TestExpandFile(t *testing.T) {
 	dest := t.TempDir()
 

pkg/chartutil/testdata/dotdotname/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v3
+name: ..
+description: A Helm chart for Kubernetes
+version: 0.1.0

pkg/chartutil/testdata/dotname/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v3
+name: .
+description: A Helm chart for Kubernetes
+version: 0.1.0

pkg/chartutil/testdata/slashinname/Chart.yaml

@@ -0,0 +1,4 @@
+apiVersion: v3
+name: a/../b
+description: A Helm chart for Kubernetes
+version: 0.1.0