diff --git a/pkg/util/http_internal_test.go b/pkg/util/http_internal_test.go new file mode 100644 index 0000000000..156749c027 --- /dev/null +++ b/pkg/util/http_internal_test.go @@ -0,0 +1,41 @@ +package util + +import ( + "bytes" + "compress/gzip" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestDecompressRequest_GzipDecompressionBomb guards the +// io.LimitReader(gzReader, maxSize+1) cap that decompressFromReader puts on gzip +// output. It feeds a tiny gzip payload that inflates to far more than maxSize (a +// "zip bomb") and asserts the buffered body never exceeds maxSize+1. +// +// This is a white-box test on purpose. The only signal that distinguishes the +// cap being present from absent is how many bytes are decompressed into memory, +// and that is not observable through ParseProtoReader: both paths return an +// error, and the compressed input is already bounded by an outer LimitReader, so +// counting bytes read from the source cannot tell them apart. Asserting on +// len(body) is deterministic and parallel-safe, unlike reading the process-wide, +// monotonic runtime.MemStats.TotalAlloc, which concurrent goroutines or parallel +// tests would inflate. Removing the cap lets the reader inflate the bomb to +// megabytes, so len(body) blows past maxSize+1 and this test fails, which is what +// makes it a real regression guard rather than a no-op. +func TestDecompressRequest_GzipDecompressionBomb(t *testing.T) { + const maxSize = 4096 // 4 KB limit on decompressed output + uncompressed := make([]byte, 32<<20) // 32 MB of zeros, compresses to a few KB + require.Greater(t, len(uncompressed), maxSize) + + var compressed bytes.Buffer + gzw := gzip.NewWriter(&compressed) + _, err := gzw.Write(uncompressed) + require.NoError(t, err) + require.NoError(t, gzw.Close()) + + body, _ := decompressRequest(bytes.NewReader(compressed.Bytes()), 0, maxSize, Gzip, nil) + + require.LessOrEqual(t, len(body), maxSize+1, + "gzip decompression buffered %d bytes, exceeding the maxSize+1 cap; the io.LimitReader guard is missing", len(body)) +} diff --git a/pkg/util/http_test.go b/pkg/util/http_test.go index d2a5e128d4..7e1668e581 100644 --- a/pkg/util/http_test.go +++ b/pkg/util/http_test.go @@ -222,25 +222,38 @@ func TestIsRequestBodyTooLargeRegression(t *testing.T) { assert.True(t, util.IsRequestBodyTooLarge(err)) } -func TestParseProtoReader_GzipDecompressionBomb(t *testing.T) { - // Create a gzip payload where decompressed size far exceeds maxSize. - const maxSize = 4096 // 4 KB limit on decompressed output - uncompressed := make([]byte, 1<<20) // 1 MB of zeros +func TestParseProtoReader_Gzip(t *testing.T) { + req := &cortexpb.PreallocWriteRequest{ + WriteRequest: cortexpb.WriteRequest{ + Timeseries: []cortexpb.PreallocTimeseries{ + {TimeSeries: &cortexpb.TimeSeries{ + Labels: []cortexpb.LabelAdapter{{Name: "foo", Value: "bar"}}, + Samples: []cortexpb.Sample{{Value: 10, TimestampMs: 1}}, + Exemplars: []cortexpb.Exemplar{}, + Histograms: []cortexpb.WrappedHistogram{}, + }}, + }, + }, + } + data, err := req.Marshal() + require.NoError(t, err) var compressed bytes.Buffer gzw := gzip.NewWriter(&compressed) - _, err := gzw.Write(uncompressed) + _, err = gzw.Write(data) require.NoError(t, err) require.NoError(t, gzw.Close()) - // The compressed payload is small enough to pass the compressed-size limit, - // but decompresses to far more than maxSize. - require.Less(t, compressed.Len(), maxSize) - + // Under-cap payload decodes successfully. maxSize must clear both the + // compressed input and the decompressed body; gzip framing makes a tiny + // message larger compressed, so size off the compressed length. + maxSize := compressed.Len() var fromWire cortexpb.PreallocWriteRequest - err = util.ParseProtoReader(context.Background(), io.NopCloser(&compressed), 0, maxSize, &fromWire, util.Gzip) - // The decompressed output should be limited to maxSize+1 bytes, causing a - // proto unmarshal error (not an OOM). The key assertion is that we don't - // allocate 1 MB of memory. - assert.NotNil(t, err) + err = util.ParseProtoReader(context.Background(), io.NopCloser(bytes.NewReader(compressed.Bytes())), 0, maxSize, &fromWire, util.Gzip) + require.NoError(t, err) + require.Equal(t, req.Timeseries, fromWire.Timeseries) + + // Capping the decompressed size below the message rejects it. + err = util.ParseProtoReader(context.Background(), io.NopCloser(bytes.NewReader(compressed.Bytes())), 0, len(data)-1, &fromWire, util.Gzip) + assert.Error(t, err) }