Skip to content

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

Open
liuyuyan2717 wants to merge 1 commit into
labstack:masterfrom
liuyuyan2717:fix-body-limit-bypass-3071
Open

fix(middleware): BodyLimit can be bypassed by a single oversized Read#3092
liuyuyan2717 wants to merge 1 commit into
labstack:masterfrom
liuyuyan2717:fix-body-limit-bypass-3071

Conversation

@liuyuyan2717

Copy link
Copy Markdown

Description

limitedReader.Read had two defects that allowed callers following standard io.Reader semantics (process n>0 bytes before checking err) to read arbitrarily far past the configured BodyLimit:

  1. Unbounded single read. Read forwarded the caller's full buffer to the underlying reader, so a single large Read could return enough data to both cross the limit and complete a value (e.g. a JSON object parsed by encoding/json.Decoder).
  2. Non-sticky error. After returning the limit-exceeded error once, subsequent Read calls continued returning real data instead of sticking at (0, err).

Fix

Mirror net/http.MaxBytesReader:

  • Bound each Read to at most remaining + 1 bytes, so a single call can never both cross the limit and complete a value.
  • Make the error sticky: once tripped, every subsequent Read returns (0, ErrStatusRequestEntityTooLarge) with no additional data.

Test coverage

Added three tests:

  • TestBodyLimitReader_stickyError — verifies post-limit reads always return 0 bytes
  • TestBodyLimitReader_boundedRead — verifies single large reads are capped at remaining+1
  • TestBodyLimit_readAfterLimitNoLeak — integration test with chunked encoding, simulating an io.Reader-compliant consumer

Fixes #3071

Checklist

  • Code compiles correctly
  • Added tests that fail without the fix
  • All existing tests should still pass

limitedReader had two defects that allowed callers following standard
io.Reader semantics (process n>0 bytes before checking err) to read
arbitrarily far past the configured limit:

1. Read forwarded the caller's full buffer to the underlying reader
   unbounded, so a single large Read could return enough data to both
   cross the limit and complete a value (e.g. a JSON object).
2. After returning the limit-exceeded error once, subsequent Read calls
   continued returning real data instead of sticking at (0, err).

Fix by mirroring net/http.MaxBytesReader:
- Bound each Read to at most remaining+1 bytes, so a single call can
  never both cross the limit and complete a value.
- Make the error sticky: once tripped, every subsequent Read returns
  (0, status 413) with no additional data.

Fixes labstack#3071
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

middleware.BodyLimit: a single oversized Read can silently bypass the limit

1 participant