From 120324c86088f2c8f5530ab4d7f15001e16af36b Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Wed, 12 Aug 2026 11:35:23 +0100
Subject: [PATCH] serve s3: reject bogus multipart part sizes in the reorder
 buffer GHSA-2p48-j3qc-rx9f

The multipart reorder-buffer admission trusted the client-declared part length.
A negative length was accepted, and `buffered + size` could overflow int64 for
a huge declared length, wrapping the running total negative and admitting
further parts past --multipart-streaming-buffer-limit.

Reject a negative length and use the overflow-safe comparison `size <=
bufferLimit - buffered` so an untrusted Content-Length can neither poison nor
overflow the budget.
---
 cmd/serve/s3/multipart.go      |  9 ++++++++-
 cmd/serve/s3/multipart_test.go | 36 ++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/cmd/serve/s3/multipart.go b/cmd/serve/s3/multipart.go
index b078c669c..11155abf9 100644
--- a/cmd/serve/s3/multipart.go
+++ b/cmd/serve/s3/multipart.go
@@ -235,14 +235,21 @@ func (b *s3Backend) UploadPart(ctx context.Context, bucketName, objectName strin
 // admitted so the sink can keep draining; so is a single part bigger than the
 // limit when the buffer is empty, to guarantee progress. Reserved bytes are
 // returned with release, or by the pump as the part is streamed.
+//
+// size is the client-declared part length and is not trusted: a negative value
+// is rejected, and the admission test is written so a huge value can't overflow
+// the running total and wrongly admit further parts past the limit.
 func (up *multipartUpload) waitForTurn(partNumber int, size int64) error {
+	if size < 0 {
+		return gofakes3.ErrInvalidArgument
+	}
 	up.mu.Lock()
 	defer up.mu.Unlock()
 	for {
 		if up.closed {
 			return gofakes3.ErrNoSuchUpload
 		}
-		if up.bufferLimit <= 0 || partNumber <= up.nextPart || up.buffered == 0 || up.buffered+size <= up.bufferLimit {
+		if up.bufferLimit <= 0 || partNumber <= up.nextPart || up.buffered == 0 || size <= up.bufferLimit-up.buffered {
 			up.buffered += size
 			return nil
 		}
diff --git a/cmd/serve/s3/multipart_test.go b/cmd/serve/s3/multipart_test.go
index 211d3ecba..5caea3e30 100644
--- a/cmd/serve/s3/multipart_test.go
+++ b/cmd/serve/s3/multipart_test.go
@@ -8,6 +8,7 @@ import (
 	"crypto/md5"
 	"fmt"
 	"io"
+	"math"
 	"net/url"
 	"path"
 	"sync"
@@ -1022,3 +1023,38 @@ func TestUploadPartNoReserveBeforeBody(t *testing.T) {
 	require.Less(t, reader.recorded, wantPages,
 		"UploadPart preallocated pool pages from the declared Content-Length before any body bytes arrived")
 }
+
+// TestWaitForTurnRejectsBogusSize checks that the reorder-buffer admission
+// rejects a negative client-declared part length and that a huge declared
+// length cannot overflow the running total so as to admit a further part past
+// the buffer limit.
+func TestWaitForTurnRejectsBogusSize(t *testing.T) {
+	up := newMultipartUpload("bucket", "key", "bucket/key", "bucket/key", nil, 1<<20)
+
+	// A part length can never be negative.
+	require.ErrorIs(t, up.waitForTurn(1, -1), gofakes3.ErrInvalidArgument)
+
+	// A huge out-of-order part is admitted once because the buffer is empty,
+	// driving buffered near the top of the int64 range.
+	require.NoError(t, up.waitForTurn(2, math.MaxInt64))
+
+	// A further out-of-order part must wait, not be wrongly admitted by an
+	// overflow of buffered+size.
+	admitted := make(chan struct{})
+	go func() {
+		_ = up.waitForTurn(3, math.MaxInt64)
+		close(admitted)
+	}()
+	select {
+	case <-admitted:
+		t.Fatal("out-of-order part admitted past the buffer limit via overflow")
+	case <-time.After(50 * time.Millisecond):
+	}
+
+	// Wake the blocked goroutine so it doesn't leak.
+	up.mu.Lock()
+	up.closed = true
+	up.cond.Broadcast()
+	up.mu.Unlock()
+	<-admitted
+}