From efc8adb0e5208ba5eb436dce95b204c82bd873c5 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 12 Aug 2026 11:26:40 +0100 Subject: [PATCH] serve s3: fix memory exhaustion from client-declared multipart part size GHSA-2p48-j3qc-rx9f CVE-PENDING Streamed multipart UploadPart called Reserve(contentLength) before reading any body bytes, so the pool immediately allocated one 1 MiB page per MiB of the client-declared Content-Length (or X-Amz-Decoded-Content-Length). An client could declare a huge part size, send no body, and force an arbitrarily large allocation without paying the bandwidth cost of the declared body. Drop the Reserve so the pool-backed buffer grows a page at a time as the body is actually read: memory now tracks the bytes received, not the unverified header. --- cmd/serve/s3/multipart.go | 5 ++-- cmd/serve/s3/multipart_test.go | 47 ++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/cmd/serve/s3/multipart.go b/cmd/serve/s3/multipart.go index 869127ba0..b078c669c 100644 --- a/cmd/serve/s3/multipart.go +++ b/cmd/serve/s3/multipart.go @@ -203,8 +203,9 @@ func (b *s3Backend) UploadPart(ctx context.Context, bucketName, objectName strin } // Buffer the part in a pool-backed RW so we can MD5 it (for the ETag) and - // stream it once it is this part's turn. - rw := multipart.NewRW().Reserve(contentLength) + // stream it once it is this part's turn. The RW grows a page at a time as + // the body is read. + rw := multipart.NewRW() hasher := md5.New() n, err := io.Copy(rw, io.TeeReader(body, hasher)) if err != nil { diff --git a/cmd/serve/s3/multipart_test.go b/cmd/serve/s3/multipart_test.go index acf36317a..211d3ecba 100644 --- a/cmd/serve/s3/multipart_test.go +++ b/cmd/serve/s3/multipart_test.go @@ -25,6 +25,7 @@ import ( "github.com/rclone/rclone/fs/operations" "github.com/rclone/rclone/fstest" "github.com/rclone/rclone/lib/multipart" + "github.com/rclone/rclone/lib/pool" "github.com/rclone/rclone/lib/random" "github.com/rclone/rclone/vfs" "github.com/rclone/rclone/vfs/vfscommon" @@ -975,3 +976,49 @@ func TestMultipartOverwrite(t *testing.T) { }) } } + +// poolProbeReader records the pool's in-use buffer count the first time it is +// read - after UploadPart has created its buffer but before any body bytes have +// been delivered - then reports a short body by returning io.EOF. +type poolProbeReader struct { + baseline int + recorded int + read bool +} + +func (r *poolProbeReader) Read(p []byte) (int, error) { + if !r.read { + r.read = true + r.recorded = pool.Global().InUse() - r.baseline + } + return 0, io.EOF +} + +// TestUploadPartNoReserveBeforeBody checks that UploadPart does not preallocate +// pool memory proportional to the client-declared Content-Length before any +// body bytes have been received. A part declaring a large size but sending no +// body must not reserve pages up front, so an unverified header cannot exhaust +// process memory. +func TestUploadPartNoReserveBeforeBody(t *testing.T) { + b, _, bucket := newPutTestBackend(t, "", nil) + ctx := context.Background() + + uploadID, err := b.CreateMultipartUpload(ctx, bucket, "object", nil) + require.NoError(t, err) + + const declared = int64(64 << 20) // 64 MiB declared by the client + wantPages := int(declared / int64(pool.BufferSize)) + + reader := &poolProbeReader{baseline: pool.Global().InUse()} + _, err = b.UploadPart(ctx, bucket, "object", uploadID, 1, declared, reader) + // No body bytes arrive, so the part is rejected as incomplete. + require.ErrorIs(t, err, gofakes3.ErrIncompleteBody) + + // The fixed path allocates nothing before the first read, so recorded is 0; + // the vulnerable path preallocated wantPages (64). The pool is process-wide, + // so recorded could pick up a few unrelated in-use buffers, but never the + // 64-page reservation the bug produced - the margin distinguishes them. + require.True(t, reader.read, "the body must have been read") + require.Less(t, reader.recorded, wantPages, + "UploadPart preallocated pool pages from the declared Content-Length before any body bytes arrived") +}