From 8fb76d6ab555577e98e23b7500009537a471feee Mon Sep 17 00:00:00 2001 From: George Jenkins Date: Fri, 6 Mar 2026 08:01:01 -0800 Subject: [PATCH] fix: Chart dot-name path bug Signed-off-by: George Jenkins --- pkg/chart/metadata.go | 3 + pkg/chart/metadata_test.go | 10 +++ pkg/chartutil/expand.go | 18 ++++ pkg/chartutil/expand_test.go | 84 +++++++++++++++++++ pkg/chartutil/testdata/dotdotname/Chart.yaml | 4 + pkg/chartutil/testdata/dotname/Chart.yaml | 4 + pkg/chartutil/testdata/slashinname/Chart.yaml | 4 + 7 files changed, 127 insertions(+) create mode 100644 pkg/chartutil/testdata/dotdotname/Chart.yaml create mode 100644 pkg/chartutil/testdata/dotname/Chart.yaml create mode 100644 pkg/chartutil/testdata/slashinname/Chart.yaml diff --git a/pkg/chart/metadata.go b/pkg/chart/metadata.go index a08a97cd1..0e78fda4e 100644 --- a/pkg/chart/metadata.go +++ b/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) } diff --git a/pkg/chart/metadata_test.go b/pkg/chart/metadata_test.go index 62aea7261..67e0b58b1 100644 --- a/pkg/chart/metadata_test.go +++ b/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"}, diff --git a/pkg/chartutil/expand.go b/pkg/chartutil/expand.go index ac59f2575..6508cb154 100644 --- a/pkg/chartutil/expand.go +++ b/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 { diff --git a/pkg/chartutil/expand_test.go b/pkg/chartutil/expand_test.go index b46ace01f..6c4f9fae6 100644 --- a/pkg/chartutil/expand_test.go +++ b/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() diff --git a/pkg/chartutil/testdata/dotdotname/Chart.yaml b/pkg/chartutil/testdata/dotdotname/Chart.yaml new file mode 100644 index 000000000..9b081f27b --- /dev/null +++ b/pkg/chartutil/testdata/dotdotname/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v3 +name: .. +description: A Helm chart for Kubernetes +version: 0.1.0 \ No newline at end of file diff --git a/pkg/chartutil/testdata/dotname/Chart.yaml b/pkg/chartutil/testdata/dotname/Chart.yaml new file mode 100644 index 000000000..597c16290 --- /dev/null +++ b/pkg/chartutil/testdata/dotname/Chart.yaml @@ -0,0 +1,4 @@ +apiVersion: v3 +name: . +description: A Helm chart for Kubernetes +version: 0.1.0 \ No newline at end of file diff --git a/pkg/chartutil/testdata/slashinname/Chart.yaml b/pkg/chartutil/testdata/slashinname/Chart.yaml new file mode 100644 index 000000000..0c522a4b6 --- /dev/null +++ b/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 \ No newline at end of file