FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(middleware): stop BodyLimit handing out more than the limit by Rohilalala · Pull Request #3072 · labstack/echo · GitHub

/ echo Public

fix(middleware): stop BodyLimit handing out more than the limit - #3072

Open
Rohilalala wants to merge 1 commit into
labstack:masterfrom
Rohilalala:fix/body-limit-bypass
Open

fix(middleware): stop BodyLimit handing out more than the limit#3072
Rohilalala wants to merge 1 commit into
labstack:masterfrom
Rohilalala:fix/body-limit-bypass

Conversation

Copy link
Copy Markdown

Fixes #3071.

limitedReader.Read passed the caller's buffer to the source untouched and only looked at the running total afterwards, and the refusal did not stick. io.Reader asks callers to process the n>0 bytes of a read before treating its error as fatal, so a caller following that advice kept getting real data on every call after the limit had already been passed.

With a 5 byte limit against a 50 byte body, reading the way the docs describe returned all 50.

Three changes, each of which the tests pin separately:

  • The read is capped at one byte past the limit. That byte is all it takes to know the body is too large, and without the cap a 64 KiB read pulls 64 KiB off the wire to enforce a 5 byte limit.
  • That byte is not handed to the caller, so the total delivered never exceeds LimitBytes.
  • Once the limit is passed the reader stays refused and does not touch the source again.

Existing BodyLimit tests are unchanged and pass.

limitedReader.Read passed the caller's buffer to the source untouched and
only looked at the running total afterwards, and the refusal did not stick.
io.Reader asks callers to process the n>0 bytes of a read before treating
its error as fatal, so a caller following that advice — encoding/json's
Decoder among them — kept getting real data on every call after the limit
had already been passed. With a 5 byte limit and a 50 byte body, 50 bytes
came through.

The read is now capped at one byte past the limit, which is all it takes to
know the body is too large; that byte is not handed to the caller; and once
the limit is passed the reader stays refused without touching the source
again.

Fixes labstack#3071
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL