From 13f6437483ae6708293c7b5e3cfcee03c05fd987 Mon Sep 17 00:00:00 2001 From: uditDewan Date: Sun, 12 Jul 2026 02:12:07 -0400 Subject: [PATCH] cli/context/store: limit decompressed size of TLS files on zip import The meta.json entry of an imported zip archive is read through limitedReader (10MB cap), but tls/ entries were read with a bare io.ReadAll. Only the compressed archive size was capped, so a small crafted zip containing a highly compressible TLS entry could decompress to gigabytes and exhaust memory. Read TLS entries through the same limitedReader already used for meta.json. This also fixes two ways in which limitedReader could silently truncate content instead of rejecting it, both of which made the cap ineffective because io.ReadAll treats io.EOF as success: - the l.N == 0 branch returned io.EOF once the budget was used up. This is reachable whenever a read lands exactly on the remaining budget, which is deterministic for zip imports since flate delivers 32KiB chunks that divide the 10MB cap evenly. - a reader is permitted to return data together with io.EOF. When such a read crossed the limit, that io.EOF was returned as-is and the limit error was never observed by the caller. The limit error now takes precedence over the underlying error as soon as the budget goes negative. True end-of-data still returns a clean io.EOF from the underlying reader. Fixes #6917 Signed-off-by: uditDewan --- cli/context/store/io_utils.go | 14 ++++++++--- cli/context/store/io_utils_test.go | 14 +++++++++++ cli/context/store/store.go | 2 +- cli/context/store/store_test.go | 40 ++++++++++++++++++++++++++++++ 4 files changed, 65 insertions(+), 5 deletions(-) diff --git a/cli/context/store/io_utils.go b/cli/context/store/io_utils.go index 097443d03174..00a852a7482a 100644 --- a/cli/context/store/io_utils.go +++ b/cli/context/store/io_utils.go @@ -5,6 +5,8 @@ import ( "io" ) +var errLimitExceeded = errors.New("read exceeds the defined limit") + // limitedReader is a fork of [io.LimitedReader] to override Read. type limitedReader struct { R io.Reader @@ -14,10 +16,7 @@ type limitedReader struct { // Read is a fork of [io.LimitedReader.Read] that returns an error when limit exceeded. func (l *limitedReader) Read(p []byte) (n int, err error) { if l.N < 0 { - return 0, errors.New("read exceeds the defined limit") - } - if l.N == 0 { - return 0, io.EOF + return 0, errLimitExceeded } // have to cap N + 1 otherwise we won't hit limit err if int64(len(p)) > l.N+1 { @@ -25,5 +24,12 @@ func (l *limitedReader) Read(p []byte) (n int, err error) { } n, err = l.R.Read(p) l.N -= int64(n) + if l.N < 0 { + // The limit must take precedence over the underlying error: a reader + // is allowed to return data together with [io.EOF], and returning that + // EOF here would make [io.ReadAll] report success for content that was + // silently truncated. + return n, errLimitExceeded + } return n, err } diff --git a/cli/context/store/io_utils_test.go b/cli/context/store/io_utils_test.go index 72cb406f853a..eacb7c9edabf 100644 --- a/cli/context/store/io_utils_test.go +++ b/cli/context/store/io_utils_test.go @@ -4,6 +4,7 @@ import ( "io" "strings" "testing" + "testing/iotest" "gotest.tools/v3/assert" ) @@ -21,4 +22,17 @@ func TestLimitReaderReadAll(t *testing.T) { r = strings.NewReader("Test") _, err = io.ReadAll(&limitedReader{R: r, N: 2}) assert.Error(t, err, "read exceeds the defined limit") + + // exceeding the limit must error even when a read lands exactly on + // the limit before the remaining data is reached + r = strings.NewReader("Test") + _, err = io.ReadAll(&limitedReader{R: iotest.OneByteReader(r), N: 2}) + assert.Error(t, err, "read exceeds the defined limit") + + // a reader is allowed to return data together with io.EOF; the limit + // must still be reported, otherwise io.ReadAll treats the truncated + // content as a successful read + r = strings.NewReader("Tes") + _, err = io.ReadAll(&limitedReader{R: iotest.DataErrReader(r), N: 2}) + assert.Error(t, err, "read exceeds the defined limit") } diff --git a/cli/context/store/store.go b/cli/context/store/store.go index 2b8b5c311478..625acab48602 100644 --- a/cli/context/store/store.go +++ b/cli/context/store/store.go @@ -478,7 +478,7 @@ func importZip(name string, s Writer, reader io.Reader) error { if err != nil { return err } - data, err := io.ReadAll(f) + data, err := io.ReadAll(&limitedReader{R: f, N: maxAllowedFileSizeToImport}) defer f.Close() if err != nil { return err diff --git a/cli/context/store/store_test.go b/cli/context/store/store_test.go index 20d0ef6463f6..3ad3ab6d9eea 100644 --- a/cli/context/store/store_test.go +++ b/cli/context/store/store_test.go @@ -236,6 +236,46 @@ func TestImportZipInvalid(t *testing.T) { assert.ErrorContains(t, err, "unexpected context file") } +func TestImportZipTLSDataTooLarge(t *testing.T) { + testDir := t.TempDir() + zf := path.Join(testDir, "test.zip") + + f, err := os.Create(zf) + assert.NilError(t, err) + defer f.Close() + w := zip.NewWriter(f) + + meta, err := json.Marshal(Metadata{ + Endpoints: map[string]any{ + "ep1": endpoint{Foo: "bar"}, + }, + Metadata: context{Bar: "baz"}, + Name: "source", + }) + assert.NilError(t, err) + mf, err := w.Create("meta.json") + assert.NilError(t, err) + _, err = mf.Write(meta) + assert.NilError(t, err) + + // a highly compressible TLS entry that inflates beyond the allowed + // import size, even though the zip file itself stays well under it + tf, err := w.Create(path.Join("tls", "docker", "ca.pem")) + assert.NilError(t, err) + _, err = tf.Write(make([]byte, 2*maxAllowedFileSizeToImport)) + assert.NilError(t, err) + err = w.Close() + assert.NilError(t, err) + + source, err := os.Open(zf) + assert.NilError(t, err) + defer source.Close() + var r io.Reader = source + s := New(testDir, testCfg) + err = Import("zipTLSTooLarge", s, r) + assert.ErrorContains(t, err, "exceeds the defined limit") +} + func TestCorruptMetadata(t *testing.T) { tempDir := t.TempDir() s := New(tempDir, testCfg)