test: make gzip decompression-bomb regression test actually bite#7655
Open
anxkhn wants to merge 1 commit into
Open
test: make gzip decompression-bomb regression test actually bite#7655anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
TestParseProtoReader_GzipDecompressionBomb only asserted that an error was returned, which passes whether or not the maxSize+1 cap on the gzip reader is present: the deferred size check in decompressRequest rejects the oversized body regardless. Assert that the call stays bounded under ~maxSize of allocation so removing the io.LimitReader cap fails the test. Add TestParseProtoReader_Gzip covering the under-cap decode and over-cap rejection, which had no gzip coverage. Fixes cortexproject#7581. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
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
1 participant
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.
What this PR does:
TestParseProtoReader_GzipDecompressionBombwas added in #7515 to guard theio.LimitReader(gzReader, maxSize+1)cap on gzip decompression, but it onlyasserted
assert.NotNil(t, err). That passes whether or not the cap is present:decompressRequest's deferredlen(body) > maxSizecheck rejects the oversizedbody regardless, so the test never exercised the new wrapper and could silently
go to a no-op if the cap were removed.
This makes the test bite by asserting decompression stays bounded: with a tiny
gzip payload that inflates to 32 MB, the call must allocate under ~1 MB, which
only holds when the inner cap stops decompression early. Removing the cap blows
past the bound and fails the test (verified: ~16.8 MB allocated vs the 1 MB
bound).
It also adds
TestParseProtoReader_Gzipcovering the under-cap decode (passes)and over-cap rejection, since the
TestParseProtoReadertable had no gzip caseat all.
Test-only change; no production code, config, or flags touched.
Which issue(s) this PR fixes:
Fixes #7581
Checklist
CHANGELOG.mdupdated (n/a, not user-facing)docs/configuration/v1-guarantees.mdupdated (n/a, no flags)