From 0a59858527aa8e132814be828b1e525fe49f1ad5 Mon Sep 17 00:00:00 2001 From: Benoit Tigeot Date: Fri, 27 Mar 2026 09:04:23 +0100 Subject: [PATCH] fix: cap directory budget reads with LimitReader Use `os.Open` + `io.LimitReader` instead of `os.ReadFile` in `ReadFileWithBudget` so a file that grows between stat and read cannot allocate unbounded memory. Also fix `MaxDecompressedFileSize` doc comment to reflect it is unused/deprecated, add nil guard on remaining, and check `os.Stat` errors in tests. Signed-off-by: Benoit Tigeot --- pkg/chart/loader/archive/archive.go | 6 +++--- pkg/chart/loader/archive/budget.go | 18 ++++++++++++++++-- pkg/chart/loader/archive/budget_test.go | 24 ++++++++++++++++++------ 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/pkg/chart/loader/archive/archive.go b/pkg/chart/loader/archive/archive.go index 0abc0e272..633871bcc 100644 --- a/pkg/chart/loader/archive/archive.go +++ b/pkg/chart/loader/archive/archive.go @@ -37,10 +37,10 @@ import ( // The default value is 100 MiB. var MaxDecompressedChartSize int64 = 100 * 1024 * 1024 // Default 100 MiB -// MaxDecompressedFileSize is the size of the largest file that Helm will attempt to load. -// The size of the file is the decompressed version of it when it is stored in an archive. +// MaxDecompressedFileSize was the per-file size limit enforced during chart loading. +// It is no longer used internally; aggregate chart size is enforced via MaxDecompressedChartSize. // -// Deprecated: This variable is no longer used internally and will be removed in Helm v5. +// Deprecated: Retained for backward compatibility with external callers. Will be removed in Helm v5. var MaxDecompressedFileSize int64 = 5 * 1024 * 1024 // Default 5 MiB var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`) diff --git a/pkg/chart/loader/archive/budget.go b/pkg/chart/loader/archive/budget.go index 93cfbfd8a..a820dec47 100644 --- a/pkg/chart/loader/archive/budget.go +++ b/pkg/chart/loader/archive/budget.go @@ -17,23 +17,37 @@ limitations under the License. package archive import ( + "errors" "fmt" + "io" "os" ) // ReadFileWithBudget reads a file and decrements remaining by the bytes read. // It returns an error if the total would exceed MaxDecompressedChartSize. +// The read is capped via io.LimitReader so a file that grows between stat +// and read cannot cause unbounded memory allocation. func ReadFileWithBudget(path string, size int64, remaining *int64) ([]byte, error) { + if remaining == nil { + return nil, errors.New("remaining budget must not be nil") + } if size > *remaining { return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) } - data, err := os.ReadFile(path) + f, err := os.Open(path) + if err != nil { + return nil, err + } + defer f.Close() + + // Read at most *remaining+1 bytes so we can detect over-budget without + // allocating unbounded memory if the file grew since stat. + data, err := io.ReadAll(io.LimitReader(f, *remaining+1)) 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) } diff --git a/pkg/chart/loader/archive/budget_test.go b/pkg/chart/loader/archive/budget_test.go index 72889638a..d8b01da97 100644 --- a/pkg/chart/loader/archive/budget_test.go +++ b/pkg/chart/loader/archive/budget_test.go @@ -46,7 +46,10 @@ func TestReadFileWithBudget(t *testing.T) { check: func(t *testing.T) { t.Helper() p := writeFile(t, "small.txt", 100) - fi, _ := os.Stat(p) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } remaining := int64(1000) data, err := ReadFileWithBudget(p, fi.Size(), &remaining) @@ -66,10 +69,13 @@ func TestReadFileWithBudget(t *testing.T) { check: func(t *testing.T) { t.Helper() p := writeFile(t, "big.txt", 500) - fi, _ := os.Stat(p) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } remaining := int64(100) - _, err := ReadFileWithBudget(p, fi.Size(), &remaining) + _, err = ReadFileWithBudget(p, fi.Size(), &remaining) if err == nil { t.Fatal("expected error for file exceeding budget") } @@ -89,7 +95,10 @@ func TestReadFileWithBudget(t *testing.T) { for i := range 3 { p := writeFile(t, fmt.Sprintf("f%d.txt", i), 80) - fi, _ := os.Stat(p) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } if _, err := ReadFileWithBudget(p, fi.Size(), &remaining); err != nil { t.Fatalf("read %d: unexpected error: %v", i, err) } @@ -99,8 +108,11 @@ func TestReadFileWithBudget(t *testing.T) { } p := writeFile(t, "over.txt", 20) - fi, _ := os.Stat(p) - _, err := ReadFileWithBudget(p, fi.Size(), &remaining) + fi, err := os.Stat(p) + if err != nil { + t.Fatalf("failed to stat %s: %v", p, err) + } + _, err = ReadFileWithBudget(p, fi.Size(), &remaining) if err == nil { t.Fatal("expected error when cumulative reads exceed budget") }