middleware.BodyLimit: a single oversized Read can silently bypass the limit
まだ誰も着手していません。
評価
調査の方向性
ミドルウェアのエントリポイントである middleware.BodyLimit が使用する limitedReader の実装から始め、次に chunked-request の例と、制限を超える量を返す reader を使って問題を再現します。制限を超えるストリーミング読み取りのリグレッションテストを追加し、制限を超えた後にデータが一切渡されないことと、設定されたエラーが引き続き観測可能であることを確認します。
索引モデルが issue の本文から書いたものです。
説明
Tested against Echo v4.15.2, reproduces on Go 1.26 and Go 1.27 alike.
limitedReader.Read forwards the caller's buffer to the underlying reader unbounded, and only checks the cumulative count afterward — critically, it's not sticky: once tripped, it keeps handing back more real data on every subsequent call:
func (r *limitedReader) Read(b []byte) (n int, err error) {
n, err = r.reader.Read(b)
r.read += int64(n)
if r.read > r.limit {
return n, echo.ErrStatusRequestEntityTooLarge
}
return
}
io.Reader's own documentation says:
Callers should always process the n > 0 bytes returned before considering the error err. Doing so correctly handles I/O errors that happen after reading some bytes and also both of the allowed EOF behaviors.
Any consumer following that documented, sanctioned pattern — including encoding/json.Decoder, which scans for a complete value before checking the trailing read error — can read arbitrarily far past the configured limit, since limitedReader keeps handing back more real data on every subsequent call after the limit has already been crossed.
Minimal reproduction, custom reader
Demonstrates the core defect directly, independent of Go version, no JSON involved: https://go.dev/play/p/vJbhmQqvI0Y
package main
import (
"bytes"
"fmt"
"net/http"
"net/http/httptest"
"github.com/labstack/echo/v4"
"github.com/labstack/echo/v4/middleware"
)
func main() {
const limit = 5 // bytes
body := bytes.Repeat([]byte("x"), 10*limit)
e := echo.New()
e.POST("/", func(c echo.Context) error {
buf := make([]byte, 64)
var total int
for {
n, err := c.Request().Body.Read(buf)
total += n
if n == 0 {
break
}
// Processing n>0 bytes before treating a non-nil err as fatal
// is exactly what io.Reader's docs say callers should do.
_ = err
}
fmt.Printf("total bytes read: %d (configured limit: %d)\n", total, limit)
return c.NoContent(http.StatusOK)
}, middleware.BodyLimit(fmt.Sprintf("%dB", limit)))
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
// Force the content-read path instead of the Content-Length fast path,
// as with a real request whose size isn't known up front (chunked).
req.ContentLength = -1
req.TransferEncoding = []string{"chunked"}
rec := httptest.NewRecorder()
e.ServeHTTP(rec, req)
}
Output:
total bytes read: 50 (configured limit: 5)
Every byte of the oversized body was delivered to the caller despite the 5-byte limit.
Encoding/json angle, for context
This is not just contrived — encoding/json.Decoder follows exactly this documented pattern, so BodyLimit + c.Bind() can silently accept oversized JSON bodies too. This is most reliably triggered on Go 1.27, where encoding/json.Decoder is backed by encoding/json/v2 by default, whose buffer-growth curve happens to align the crossing-the-limit read with completion of the value for common payload sizes — but as the reproduction above shows, the underlying issue is not Go-version-specific.
Minimal reproduction, JSON: https://go.dev/play/p/x9suEwdIO2x
Suggested fix
Mirror net/http.MaxBytesReader. Two changes are both required — bounding the read alone is not enough, since a caller that keeps calling Read after getting an error alongside n > 0 can still assemble the full body one small chunk at a time:
-
Bound the request size. Never ask the underlying reader for more than
remaining+1bytes, so a singleReadcan never return enough data to both cross the limit and complete a value:remaining := r.limit - r.read if int64(len(b))-1 > remaining { b = b[:remaining+1] } -
Make it sticky. Once the limit is crossed, permanently return
(0, err)on every subsequent call — never hand back more real data, no matter how many more times the caller retries:func (r *limitedReader) Read(b []byte) (n int, err error) { if r.err != nil { return 0, r.err } ... // bound as above, then on overflow: r.err = echo.ErrStatusRequestEntityTooLarge }
Happy to submit a PR with this change if it's welcome.
- 主要言語
- Go
- スター
- 32.7k
- フォーク
- 2.8k
- 平均マージ
- 9時間 39分
- マージ済み PR(30日)
- 6
コントリビューションガイド
このリポジトリのコントリビューションガイドは索引されていません
はじめの一歩
- issue を最後まで読み、次にプロジェクトのコントリビューションガイドを読みます。
- 着手することを issue にコメントします — 二人が同じ作業をするのを防げます。
- リポジトリをフォークし、ブランチを切って変更します。
- issue 番号を参照したプルリクエストを送ります。
labstack/echo のほかの issue
-
難易度 2/5 1〜3時間 初心者へのやさしさ 74/100
-
難易度 3/5 1〜2日 初心者へのやさしさ 65/100
-
難易度 3/5 1〜2日 初心者へのやさしさ 68/100
-
難易度 5/5 1週間以上 初心者へのやさしさ 35/100
-
難易度 4/5 3〜5日 初心者へのやさしさ 48/100
似ている issue
-
難易度 1/5 1時間未満 初心者へのやさしさ 60/100
github/gh-aw-mcpg#13748 ·
-
agentic-workflows
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
-
needs-triage
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 65/100
-
難易度 2/5 1〜3時間 初心者へのやさしさ 75/100
googleapis/librarian#7670 · コメント 2 件 ·