fix(middleware): BodyLimit can be bypassed by a single oversized Read

middleware/body_limit.go

@@ -24,6 +24,7 @@ type limitedReader struct {
 	BodyLimitConfig
 	reader io.ReadCloser
 	read   int64
+	err    error
 }
 
 // BodyLimit returns a BodyLimit middleware.
@@ -81,10 +82,27 @@ func (config BodyLimitConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
 }
 
 func (r *limitedReader) Read(b []byte) (n int, err error) {
+	// Once the limit has been exceeded, make it sticky — always return 0 bytes and the error.
+	// This prevents callers (like encoding/json.Decoder, which processes n>0 before checking err)
+	// from reading arbitrarily far past the limit by repeatedly calling Read.
+	if r.err != nil {
+		return 0, r.err
+	}
+
+	// Bound the read size so a single Read call can never return enough bytes to both
+	// cross the limit and complete a value (e.g. a JSON object).
+	// We allow one byte past the limit so the caller gets the error alongside the data
+	// that caused the overflow, which matches standard io.Reader semantics.
+	remaining := r.LimitBytes - r.read
+	if remaining < int64(len(b))-1 {
+		b = b[:remaining+1]
+	}
+
 	n, err = r.reader.Read(b)
 	r.read += int64(n)
 	if r.read > r.LimitBytes {
-		return n, echo.ErrStatusRequestEntityTooLarge
+		r.err = echo.ErrStatusRequestEntityTooLarge
+		return n, r.err
 	}
 	return
 }
@@ -96,4 +114,5 @@ func (r *limitedReader) Close() error {
 func (r *limitedReader) Reset(reader io.ReadCloser) {
 	r.reader = reader
 	r.read = 0
+	r.err = nil
 }

middleware/body_limit_test.go

@@ -189,3 +189,108 @@ func TestBodyLimit(t *testing.T) {
 	assert.Equal(t, http.StatusOK, rec.Code)
 	assert.Equal(t, hw, rec.Body.Bytes())
 }
+
+func TestBodyLimitReader_stickyError(t *testing.T) {
+	// Verifies that once the limit is exceeded, subsequent Read calls always
+	// return (0, err) — no more data is leaked to the caller.
+	// This is the fix for issue #3071 where callers following the standard
+	// io.Reader pattern (process n>0 before checking err) could read arbitrarily
+	// far past the limit.
+	hw := []byte("Hello, World!")
+
+	config := BodyLimitConfig{
+		Skipper:    DefaultSkipper,
+		LimitBytes: 5,
+	}
+	reader := &limitedReader{
+		BodyLimitConfig: config,
+		reader:          io.NopCloser(bytes.NewReader(hw)),
+	}
+
+	buf := make([]byte, 1)
+
+	// Read 5 bytes one at a time — should all succeed with no error
+	for i := 0; i < 5; i++ {
+		n, err := reader.Read(buf)
+		assert.Equal(t, 1, n)
+		assert.NoError(t, err)
+	}
+
+	// 6th read should cross the limit and return the error alongside 1 byte
+	n, err := reader.Read(buf)
+	assert.Equal(t, 1, n)
+	he := err.(echo.HTTPStatusCoder)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+
+	// All subsequent reads must return (0, err) — never more data
+	for i := 0; i < 10; i++ {
+		n, err = reader.Read(buf)
+		assert.Equal(t, 0, n, "read after limit exceeded must return 0 bytes")
+		he = err.(echo.HTTPStatusCoder)
+		assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+	}
+}
+
+func TestBodyLimitReader_boundedRead(t *testing.T) {
+	// Verifies that a single large Read buffer can never return enough bytes to
+	// both cross the limit and complete a value — only remaining+1 bytes max.
+	// This prevents a single oversized Read from silently bypassing the limit.
+	hw := bytes.Repeat([]byte("x"), 100)
+
+	config := BodyLimitConfig{
+		Skipper:    DefaultSkipper,
+		LimitBytes: 10,
+	}
+	reader := &limitedReader{
+		BodyLimitConfig: config,
+		reader:          io.NopCloser(bytes.NewReader(hw)),
+	}
+
+	// A large buffer read should return at most remaining+1 = 11 bytes, not the full 100
+	buf := make([]byte, 100)
+	n, err := reader.Read(buf)
+	assert.LessOrEqual(t, n, 11, "single read must not return more than remaining+1 bytes")
+	he := err.(echo.HTTPStatusCoder)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+}
+
+func TestBodyLimit_readAfterLimitNoLeak(t *testing.T) {
+	// Integration test: verifies that BodyLimit middleware correctly enforces the
+	// limit even when the handler reads with a large buffer and processes n>0
+	// bytes before checking err (standard io.Reader semantics).
+	e := echo.New()
+
+	const limit = 5
+	body := bytes.Repeat([]byte("x"), 10*limit) // 50 bytes, well over the limit
+
+	req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
+	// Force chunked encoding to bypass the Content-Length fast path
+	req.ContentLength = -1
+	req.TransferEncoding = []string{"chunked"}
+	rec := httptest.NewRecorder()
+	c := e.NewContext(req, rec)
+
+	mw := BodyLimit(limit)
+	h := func(c *echo.Context) error {
+		// Simulate a handler that follows io.Reader semantics exactly:
+		// process n>0 bytes before considering err fatal.
+		// (This is exactly what encoding/json.Decoder does.)
+		buf := make([]byte, 64)
+		var total int
+		for {
+			n, err := c.Request().Body.Read(buf)
+			total += n
+			if n == 0 {
+				break
+			}
+			_ = err // process data first, per io.Reader docs
+		}
+		// The handler must never be able to read more than limit+1 bytes total.
+		assert.LessOrEqual(t, total, limit+1, "handler must not read past limit+1")
+		return nil
+	}
+
+	err := mw(h)(c)
+	he := err.(echo.HTTPStatusCoder)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+}