From bb213c06afcae8492cb2eb3ae59badc8eb12ec9b Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 27 Mar 2026 08:29:17 +0100 Subject: [PATCH] fix: enforce aggregate size budget on directory loading Directory-based chart loading (`LoadDir`) used unbounded `os.ReadFile` calls with no total size check. Archive loading already enforces `MaxDecompressedChartSize` via a remaining-byte budget but directory loading did not, leaving local charts and `file://` dependencies as an unbounded memory path. Add `ReadFileWithBudget` in the archive package and use it in both v2 and v3 directory loaders so they track the same aggregate budget. Ref: https://github.com/helm/helm/pull/31748#issuecomment-4138927643 Signed-off-by: Benoit Tigeot --- internal/chart/v3/loader/directory.go | 3 +- pkg/chart/loader/archive/budget.go | 43 +++++++++ pkg/chart/loader/archive/budget_test.go | 114 ++++++++++++++++++++++++ pkg/chart/v2/loader/directory.go | 3 +- pkg/chart/v2/loader/load_test.go | 14 +++ 5 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 pkg/chart/loader/archive/budget.go create mode 100644 pkg/chart/loader/archive/budget_test.go diff --git a/internal/chart/v3/loader/directory.go b/internal/chart/v3/loader/directory.go index 5937efda9..bc9a23ce1 100644 --- a/internal/chart/v3/loader/directory.go +++ b/internal/chart/v3/loader/directory.go @@ -64,6 +64,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files := []*archive.BufferedFile{} topdir += string(filepath.Separator) + remaining := archive.MaxDecompressedChartSize walk := func(name string, fi os.FileInfo, err error) error { n := strings.TrimPrefix(name, topdir) @@ -100,7 +101,7 @@ func LoadDir(dir string) (*chart.Chart, error) { return fmt.Errorf("cannot load irregular file %s as it has file mode type bits set", name) } - data, err := os.ReadFile(name) + data, err := archive.ReadFileWithBudget(name, fi.Size(), &remaining) if err != nil { return fmt.Errorf("error reading %s: %w", n, err) } diff --git a/pkg/chart/loader/archive/budget.go b/pkg/chart/loader/archive/budget.go new file mode 100644 index 000000000..93cfbfd8a --- /dev/null +++ b/pkg/chart/loader/archive/budget.go @@ -0,0 +1,43 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package archive + +import ( + "fmt" + "os" +) + +// ReadFileWithBudget reads a file and decrements remaining by the bytes read. +// It returns an error if the total would exceed MaxDecompressedChartSize. +func ReadFileWithBudget(path string, size int64, remaining *int64) ([]byte, error) { + if size > *remaining { + return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) + } + + data, err := os.ReadFile(path) + if err != nil { + return nil, err + } + + // Re-check with actual length: the file may have grown between stat and read. + if int64(len(data)) > *remaining { + return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) + } + + *remaining -= int64(len(data)) + return data, nil +} diff --git a/pkg/chart/loader/archive/budget_test.go b/pkg/chart/loader/archive/budget_test.go new file mode 100644 index 000000000..72889638a --- /dev/null +++ b/pkg/chart/loader/archive/budget_test.go @@ -0,0 +1,114 @@ +/* +Copyright The Helm Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package archive + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +func TestReadFileWithBudget(t *testing.T) { + dir := t.TempDir() + + writeFile := func(t *testing.T, name string, size int) string { + t.Helper() + p := filepath.Join(dir, name) + if err := os.WriteFile(p, make([]byte, size), 0644); err != nil { + t.Fatal(err) + } + return p + } + + expectedErr := fmt.Sprintf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) + + tcs := []struct { + name string + check func(t *testing.T) + }{ + { + name: "reads file and decrements budget", + check: func(t *testing.T) { + t.Helper() + p := writeFile(t, "small.txt", 100) + fi, _ := os.Stat(p) + remaining := int64(1000) + + data, err := ReadFileWithBudget(p, fi.Size(), &remaining) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(data) != 100 { + t.Fatalf("expected 100 bytes, got %d", len(data)) + } + if remaining != 900 { + t.Fatalf("expected remaining=900, got %d", remaining) + } + }, + }, + { + name: "rejects file exceeding budget", + check: func(t *testing.T) { + t.Helper() + p := writeFile(t, "big.txt", 500) + fi, _ := os.Stat(p) + remaining := int64(100) + + _, err := ReadFileWithBudget(p, fi.Size(), &remaining) + if err == nil { + t.Fatal("expected error for file exceeding budget") + } + if err.Error() != expectedErr { + t.Fatalf("expected %q, got %q", expectedErr, err.Error()) + } + if remaining != 100 { + t.Fatalf("budget should not change on rejection, got %d", remaining) + } + }, + }, + { + name: "tracks budget across multiple reads", + check: func(t *testing.T) { + t.Helper() + remaining := int64(250) + + for i := range 3 { + p := writeFile(t, fmt.Sprintf("f%d.txt", i), 80) + fi, _ := os.Stat(p) + if _, err := ReadFileWithBudget(p, fi.Size(), &remaining); err != nil { + t.Fatalf("read %d: unexpected error: %v", i, err) + } + } + if remaining != 10 { + t.Fatalf("expected remaining=10, got %d", remaining) + } + + p := writeFile(t, "over.txt", 20) + fi, _ := os.Stat(p) + _, err := ReadFileWithBudget(p, fi.Size(), &remaining) + if err == nil { + t.Fatal("expected error when cumulative reads exceed budget") + } + }, + }, + } + + for _, tc := range tcs { + t.Run(tc.name, tc.check) + } +} diff --git a/pkg/chart/v2/loader/directory.go b/pkg/chart/v2/loader/directory.go index e213a0da8..da6b275d5 100644 --- a/pkg/chart/v2/loader/directory.go +++ b/pkg/chart/v2/loader/directory.go @@ -64,6 +64,7 @@ func LoadDir(dir string) (*chart.Chart, error) { files := []*archive.BufferedFile{} topdir += string(filepath.Separator) + remaining := archive.MaxDecompressedChartSize walk := func(name string, fi os.FileInfo, err error) error { n := strings.TrimPrefix(name, topdir) @@ -100,7 +101,7 @@ func LoadDir(dir string) (*chart.Chart, error) { return fmt.Errorf("cannot load irregular file %s as it has file mode type bits set", name) } - data, err := os.ReadFile(name) + data, err := archive.ReadFileWithBudget(name, fi.Size(), &remaining) if err != nil { return fmt.Errorf("error reading %s: %w", n, err) } diff --git a/pkg/chart/v2/loader/load_test.go b/pkg/chart/v2/loader/load_test.go index aed071b2f..5fd844f38 100644 --- a/pkg/chart/v2/loader/load_test.go +++ b/pkg/chart/v2/loader/load_test.go @@ -51,6 +51,20 @@ func TestLoadDir(t *testing.T) { verifyDependenciesLock(t, c) } +func TestLoadDirExceedsBudget(t *testing.T) { + orig := archive.MaxDecompressedChartSize + archive.MaxDecompressedChartSize = 1 // 1 byte budget + defer func() { archive.MaxDecompressedChartSize = orig }() + + _, err := LoadDir("testdata/frobnitz") + if err == nil { + t.Fatal("expected error when chart directory exceeds budget") + } + if !strings.Contains(err.Error(), "chart exceeds maximum decompressed size") { + t.Fatalf("unexpected error: %v", err) + } +} + func TestLoadDirWithDevNull(t *testing.T) { if runtime.GOOS == "windows" { t.Skip("test only works on unix systems with /dev/null present")