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 <benoit.tigeot@lifen.fr>
pull/31748/head
Benoit Tigeot 7 months ago
parent bb213c06af
commit 0a59858527
No known key found for this signature in database
GPG Key ID: 8E6D4FC8AEBDA62C

@ -37,10 +37,10 @@ import (
// The default value is 100 MiB. // The default value is 100 MiB.
var MaxDecompressedChartSize int64 = 100 * 1024 * 1024 // Default 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. // MaxDecompressedFileSize was the per-file size limit enforced during chart loading.
// The size of the file is the decompressed version of it when it is stored in an archive. // 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 MaxDecompressedFileSize int64 = 5 * 1024 * 1024 // Default 5 MiB
var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`) var drivePathPattern = regexp.MustCompile(`^[a-zA-Z]:/`)

@ -17,23 +17,37 @@ limitations under the License.
package archive package archive
import ( import (
"errors"
"fmt" "fmt"
"io"
"os" "os"
) )
// ReadFileWithBudget reads a file and decrements remaining by the bytes read. // ReadFileWithBudget reads a file and decrements remaining by the bytes read.
// It returns an error if the total would exceed MaxDecompressedChartSize. // 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) { 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 { if size > *remaining {
return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) 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 { if err != nil {
return nil, err return nil, err
} }
// Re-check with actual length: the file may have grown between stat and read.
if int64(len(data)) > *remaining { if int64(len(data)) > *remaining {
return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize) return nil, fmt.Errorf("chart exceeds maximum decompressed size of %d bytes", MaxDecompressedChartSize)
} }

@ -46,7 +46,10 @@ func TestReadFileWithBudget(t *testing.T) {
check: func(t *testing.T) { check: func(t *testing.T) {
t.Helper() t.Helper()
p := writeFile(t, "small.txt", 100) 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) remaining := int64(1000)
data, err := ReadFileWithBudget(p, fi.Size(), &remaining) data, err := ReadFileWithBudget(p, fi.Size(), &remaining)
@ -66,10 +69,13 @@ func TestReadFileWithBudget(t *testing.T) {
check: func(t *testing.T) { check: func(t *testing.T) {
t.Helper() t.Helper()
p := writeFile(t, "big.txt", 500) 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) remaining := int64(100)
_, err := ReadFileWithBudget(p, fi.Size(), &remaining) _, err = ReadFileWithBudget(p, fi.Size(), &remaining)
if err == nil { if err == nil {
t.Fatal("expected error for file exceeding budget") t.Fatal("expected error for file exceeding budget")
} }
@ -89,7 +95,10 @@ func TestReadFileWithBudget(t *testing.T) {
for i := range 3 { for i := range 3 {
p := writeFile(t, fmt.Sprintf("f%d.txt", i), 80) 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 { if _, err := ReadFileWithBudget(p, fi.Size(), &remaining); err != nil {
t.Fatalf("read %d: unexpected error: %v", i, err) t.Fatalf("read %d: unexpected error: %v", i, err)
} }
@ -99,8 +108,11 @@ func TestReadFileWithBudget(t *testing.T) {
} }
p := writeFile(t, "over.txt", 20) p := writeFile(t, "over.txt", 20)
fi, _ := os.Stat(p) fi, err := os.Stat(p)
_, err := ReadFileWithBudget(p, fi.Size(), &remaining) if err != nil {
t.Fatalf("failed to stat %s: %v", p, err)
}
_, err = ReadFileWithBudget(p, fi.Size(), &remaining)
if err == nil { if err == nil {
t.Fatal("expected error when cumulative reads exceed budget") t.Fatal("expected error when cumulative reads exceed budget")
} }

Loading…
Cancel
Save