GODRIVER-4024 fix: validate UncompressedSize before allocation in DecompressPayload#2479
Open
iabdullah215 wants to merge 2 commits into
Open
Conversation
DecompressPayload used opts.UncompressedSize -- which is read unvalidated off the wire via wiremessage.ReadCompressedUncompressedSize in Operation.decompressWireMessage -- directly as an allocation size. A malformed or malicious OP_COMPRESSED frame could therefore set it to a large value (up to math.MaxInt32) to drive a ~2GB allocation (OOM), or to a negative value to trigger a make([]byte, negative) runtime panic. Reject sizes < 0 or > 48000000 (the wire protocol maximum message size, defaultMaxMessageSize) at the top of DecompressPayload, before any allocation on the zlib, zstd, and snappy paths. Add regression tests to TestDecompressFailures covering oversized and negative UncompressedSize values for zlib, zstd, and snappy.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the OP_COMPRESSED decompression path by validating the server-provided UncompressedSize before it can be used to allocate buffers in DecompressPayload, preventing OOM or panics from malformed/malicious wire data.
Changes:
- Add a
maxDecompressedSizelimit and reject negative/oversizedUncompressedSizevalues before any decompression allocation occurs. - Extend
TestDecompressFailureswith regression cases for out-of-rangeUncompressedSizevalues across compressor IDs.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| x/mongo/driver/compression.go | Adds a max-size constant and validates UncompressedSize before allocating during decompression. |
| x/mongo/driver/compression_test.go | Adds regression tests for invalid UncompressedSize values to ensure clean errors instead of allocation/panic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Include the allowed range (0..maxDecompressedSize) in the invalid-size error so the rejected value is easier to diagnose. Add a "snappy oversized size" case to TestDecompressFailures so the out-of-range check is exercised for all three compressor IDs.
Contributor
|
@iabdullah215 Thanks for the improvement! The Go Driver team is at capacity right now, but we've scheduled reviewing your PR after some pressing work. Someone from the team will review it in about a month. |
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.
GODRIVER-4024
DecompressPayloadusesopts.UncompressedSizedirectly as an allocation size without validating it first. That value is read straight off the wire and is fully controlled by the server, so a malformed or maliciousOP_COMPRESSEDframe can force the driver into an unbounded allocation or a runtime panic. This PR adds a bounds check before any allocation occurs.Root Cause
The size flows unvalidated from the wire all the way to the allocation site:
Inside
DecompressPayload, the server-controlled value is used directly as an allocation size with no validation on any of the three compressor paths:The existing wire-level
maxMessageSizecheck (48 MB, enforced intopology/connection.go) applies to the size of the compressed message on the wire, not to the declared uncompressed size. A server can therefore send a small, valid compressed message that passes the wire-size check while claiming an arbitrarily large uncompressed size, and the driver will attempt the allocation before decompressing a single byte.Impact
A server, or an on-path attacker on a connection where compression is negotiated, can send an
OP_COMPRESSEDmessage with a craftedUncompressedSizefield and trigger one of two outcomes:Unbounded allocation / OOM. A large positive value, up to
math.MaxInt32(2,147,483,647), drives a roughly 2 GB allocation from a few bytes of compressed input. The subsequent read fails because the actual data does not fill the buffer, but the allocation has already happened and the client process is killed by the OS out-of-memory handler before it can return an error.Panic / client crash. A negative value, for example wire bytes
0xFFFFFFFF, causesmake([]byte, negative)to panic withmakeslice: len out of range. This panic is unrecovered in the message-read path and terminates the client process.Impact is availability only. There is no memory disclosure and no path to remote code execution. The
int32type caps a single allocation at approximately 2 GB. This is a robustness and hardening fix.Fix
Reject out-of-range sizes at the top of
DecompressPayload, before any allocation occurs on the zlib, zstd, or snappy paths:maxDecompressedSizeis set to 48,000,000 bytes, matchingdefaultMaxMessageSizealready established inx/mongo/driver/topology/connection.go. A legitimate uncompressed message can never exceed 48 MB under the MongoDB wire protocol, so no valid traffic is rejected by this check.Testing
Added regression cases to
TestDecompressFailurescovering oversized and negativeUncompressedSizefor all three compressor paths: zlib, zstd, and snappy. Each case now returns a clean error instead of allocating or panicking. Existing round-trip tests (TestCompression,TestCompressionLevels) continue to pass unchanged.Backward Compatibility
No public API change. The signatures of
DecompressPayloadandCompressionOptsare unchanged. The behavior change is limited to previously-invalid inputs, specifically sizes below zero or above 48 MB, which now return an error instead of triggering an unbounded allocation or a runtime panic.