fix(http): cap decompressed body to MaxBodySize to block gzip bombs#886
Open
Shinku-Chen wants to merge 1 commit into
Conversation
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The body-size limit applied io.LimitReader to the raw response body
before gzip.NewReader, so MaxBodySize only bounded the compressed
bytes pulled from the network. A gzip-bombed response — small
compressed payload (KBs) that decompresses to many GBs of repeated
bytes — passed through the limit untouched: the decompressed
io.ReadAll then exhausted memory. This is a classic DoS shape when
scraping untrusted hosts.
Move LimitReader to after the gzip wrap so it caps the decompressed
stream. Non-gzip responses are still bounded because bodyReader
remains res.Body in that path. For gzip responses, the gzip reader
stops pulling more compressed bytes once the LimitReader sees
MaxBodySize decompressed bytes, so both memory and bandwidth remain
bounded.
Add TestMaxBodySizeCapsDecompressedGzipBody: serve ~1 KiB of
gzip-compressed zeros that expand to 1 MiB, set MaxBodySize to 4 KiB,
and assert the body received by OnResponse is at most 4 KiB. The
test reports "body length 1048576 exceeds MaxBodySize 4096" on the
previous implementation and passes with the fix. DisableCompression
on the test transport prevents net/http from gunzipping internally
so the colly gzip path is actually exercised.