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.
This commit is contained in:
Nick Craig-Wood
2026-09-04 19:00:22 +01:00
parent 935197b062
commit efc8adb0e5
2 changed files with 50 additions and 2 deletions
+3 -2
View File
@@ -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 {
+47
View File
@@ -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")
}