SERVER-116125 Add zlib decompression validation #1628

ravisastryk · 2026-01-05T01:56:14Z

Summary

Adds input validation and security hardening to ZlibMessageCompressor::decompressData() as defense-in-depth for CVE-2025-14847. Fixes metrics bug in counterHitDecompress().

Changes

Add minimum payload size check (≥8 bytes)
Add zlib header validation (CMF/FLG checksum)
Add zip bomb protection (max 1024:1 ratio)
Fix counterHitDecompress() to use actual length
Add additional regression tests to make sure there are no breaking changes

Testing

Existing tests pass
New regression tests pass
Valid compression unaffected

References

CVE: https://nvd.nist.gov/vuln/detail/CVE-2025-14847
JIRA: SERVER-115508

Contributor: Ravi Sastry Kadali (@ravisastryk)

…E-2025-14847)

ravisastryk · 2026-01-05T02:09:04Z

@spencerjackson @BillyDonahue I have implemented few additional defense-in-depth hardening around zlib compression just to reinforce the CVE-2025-14847 fix. When you get a chance, please review so we can move forward with hardening MongoDB for wider adoption.

src/mongo/transport/message_compressor_zlib.cpp

BillyDonahue · 2026-01-05T20:01:58Z

src/mongo/transport/message_compressor_zlib.cpp

+    // FIX: Use actual decompressed length for metrics
+    // Previous code incorrectly used output.length() (buffer size)


Thanks for this fix. Actually this should be the headline and focus of its own PR and ticket.
History of what a line of code used to be is essentially irrelevant. Remove comments about it.
Just fix it and commit it.

BillyDonahue · 2026-01-05T20:14:20Z

src/mongo/transport/message_compressor_manager_test.cpp

+ * CVE-2025-14847 defense-in-depth: minimum payload size validation
+ */
+TEST(ZlibMessageCompressor, CVE2025_14847_RejectsUndersizedPayload) {


I think it's better to just test to assert properties without referencing a CVE code.
The name (with prefix removed) describes the test sufficiently. No comment needed at all.

BillyDonahue · 2026-01-05T20:24:37Z

src/mongo/transport/message_compressor_manager_test.cpp

+ * Test: Excessive decompression ratio is rejected (zip bomb protection)
+ * CVE-2025-14847 defense-in-depth: ratio limit validation
+ */
+TEST(ZlibMessageCompressor, CVE2025_14847_RejectsZipBomb) {


Why would an output buffer that happens to have a large capacity be cause for alarm?
The output capacity isn't determined by the user-specified payload AFAICT.

Please answer this. I don't understand the problem.

You are right, this ratio check creates false positives without adding meaningful security. I have removed it entirely.

src/mongo/transport/message_compressor_manager_test.cpp

BillyDonahue · 2026-01-05T21:04:44Z

To accept this code, we'd need to get a contributor's agreement from you.
https://github.com/mongodb/mongo/wiki#contributing-to-mongodb

- Update assertZlibDecompressBadValue to accept optional output buffer size parameter - Simplify RejectsExcessiveDecompressionRatio test using the enhanced helper - Eliminates code duplication, makes test intent clearer

- Use char* for header validation, hide Bytef abstraction - Replace magic number 8 with Z_DEFLATED constant - Remove numbered comments and braces from one-liner if statements - Fix counterHitDecompress to use actual decompressed length (not buffer size) - Compact error messages

ravisastryk · 2026-01-06T00:58:51Z

To accept this code, we'd need to get a contributor's agreement from you. https://github.com/mongodb/mongo/wiki#contributing-to-mongodb
Thank you @BillyDonahue for reviewing the PR and offering suggestions.
I have signed contributor's agreement. Please re-review when you get a chance

BillyDonahue · 2026-01-06T07:22:28Z

src/mongo/transport/message_compressor_zlib.cpp

+// Per RFC1950: validate first two bytes (CMF, FLG). CM (CMF & 0x0F) must be
+// Z_DEFLATED (8) and (CMF*256 + FLG) must be a multiple of 31.


We're saying that already below. I don't think this level of detail should be in the function docs.
We can not have any comment above this function, as its name is sufficient.

BillyDonahue · 2026-01-06T07:23:59Z

src/mongo/transport/message_compressor_zlib.cpp

+// Maximum allowed decompression ratio to prevent zip bombs
+// A ratio above 1024:1 is suspicious for typical MongoDB messages


I still disagree with this check. I don't understand the ratio being suspicious if it isn't controlled by incoming queries at all. Isn't output capacity just chosen by the calling function?

Your reasoning makes complete sense. The output buffer size is allocated by the calling code based on message metadata, not controlled by malicious input. Checking the ratio creates a false sense of security and could cause false positives when legitimate buffers are reused or pre-allocated for worst-case scenarios. The real protection is already in place through header validation and minimum size checks. I am proceeding with removing decompression ratio check altogether.

BillyDonahue · 2026-01-06T07:24:36Z

src/mongo/transport/message_compressor_zlib.cpp

+    if (output.length() > input.length() * kMaxDecompressionRatio)
+        return Status{ErrorCodes::BadValue, "Decompression ratio too large"};


I need an explanation of why this ratio should be enforced.

You have a valid point. Than you for catching. The output buffer size is allocated by the calling code based on message metadata, not controlled by malicious input. Checking the ratio creates a false sense of security and could cause false positives when legitimate buffers are reused or pre-allocated for worst-case scenarios. The real protection is already in place through header validation and minimum size checks. We would not need to enforce this check.

BillyDonahue · 2026-01-06T07:26:01Z

src/mongo/transport/message_compressor_manager_test.cpp

    ASSERT_EQ(ErrorCodes::BadValue, swm.getStatus());
 }

+// Helper: assert that decompression of payload fails with BadValue.


Remove this line of the comment as it's restating the function name.
The following line should be commented with /** */ comments

BillyDonahue · 2026-01-06T07:27:27Z

src/mongo/transport/message_compressor_manager_test.cpp

+void assertZlibDecompressBadValue(const std::vector<char>& payload, size_t outputSize = 1024) {
+    auto compressor = std::make_unique<ZlibMessageCompressor>();
+    ConstDataRange input(payload.data(), payload.size());
+


no blank lines in this function. I'm not sure what they're delineating. Seems unnecessary.

BillyDonahue · 2026-01-06T07:30:01Z

src/mongo/transport/message_compressor_manager_test.cpp

+    ASSERT_NOT_OK(result);
+    ASSERT_EQ(result.getStatus().code(), ErrorCodes::BadValue);


The callers should do the ASSERT calls.
Then they can determine what they want to check for.
The caller should also be given an opportunity to pass in an output vector (rather than just its size), so they can examine the decompressed data.

Then this function could be used for non-failure cases.
I think we just need something to swallow the decompression call syntax noise. But take a vector of input and produce a vector of output and a status and an output size.

If we did an output.resize(result.getValue()), that should be a way to get the output size back to the caller. We can just return a StatusWith<std::vector> output vector and they can see its .size().

Thank you for highlighting this aspect. Totally makes sense.

BillyDonahue · 2026-01-06T07:33:26Z

src/mongo/transport/message_compressor_manager_test.cpp


+TEST(ZlibMessageCompressor, RejectsUndersizedPayload) {
+    // Payload smaller than kMinZlibPayloadSize (8 bytes)
+    assertZlibDecompressBadValue(std::vector<char>{0x78, 0x9c, 0x03, 0x00});


You don't need the vector type to be mentioned. Just use its initializer list constructor.

Suggested change

assertZlibDecompressBadValue(std::vector<char>{0x78, 0x9c, 0x03, 0x00});

assertZlibDecompressBadValue({0x78, 0x9c, 0x03, 0x00});

BillyDonahue · 2026-01-06T07:37:43Z

src/mongo/transport/message_compressor_manager_test.cpp

+TEST(ZlibMessageCompressor, RejectsInvalidHeader) {
+    // Invalid header: wrong compression method (not deflate)
+    assertZlibDecompressBadValue(std::vector<char>{
+        0x70, 0x9c,  // Invalid CMF (method != Z_DEFLATED)


The CMF is the 0x70, right? So the comment should be next to the 0x70.
But I think the comment should probably move up above the statement, rather than interrupting the data.

BillyDonahue · 2026-01-06T07:37:56Z

src/mongo/transport/message_compressor_manager_test.cpp

+}
+
+TEST(ZlibMessageCompressor, RejectsInvalidHeader) {
+    // Invalid header: wrong compression method (not deflate)


redundant. omit

BillyDonahue · 2026-01-06T07:38:22Z

src/mongo/transport/message_compressor_manager_test.cpp

+
+TEST(ZlibMessageCompressor, RejectsExcessiveDecompressionRatio) {
+    // Valid zlib header but request an unreasonably large output capacity (>1024x input).
+    std::vector<char> smallPayload = {0x78, 0x9c, 0x63, 0x60, 0x00, 0x00, 0x00, 0x02, 0x00, 0x01};


just inline this into the decompress call

BillyDonahue · 2026-01-06T07:38:41Z

src/mongo/transport/message_compressor_manager_test.cpp

+}
+
+TEST(ZlibMessageCompressor, RejectsEmptyPayload) {
+    assertZlibDecompressBadValue(std::vector<char>{});


Suggested change

assertZlibDecompressBadValue(std::vector<char>{});

assertZlibDecompressBadValue({});

Changing assertZlibDecompressBadValue to zlibDecompress instead so that it returns StatusWith<size_t> for caller flexibility and callers can do their own assertions and can be leveraged for both success and failure cases.

BillyDonahue · 2026-01-06T07:39:14Z

src/mongo/transport/message_compressor_manager_test.cpp

+    DataRange decompressedRange(decompressed.data(), decompressed.size());
+
+    ConstDataRange compressedInput(compressed.data(), compressResult.getValue());
+    auto decompressResult = compressor->decompressData(compressedInput, decompressedRange);


we have a helper function to handle this decompression.

BillyDonahue · 2026-01-06T21:20:10Z

I'm generating a lot of feedback here. In the interest of time, I'm going to take this PR and make an internal one as a rebase of it. I'll omit the "large output buffer" check but keep the others.

- Fix operator precedence bug in header validation (cmf & 0xf) - Remove decompression ratio check per reviewer feedback - Refactor test helper to return StatusWith for caller flexibility - Clean up style: remove verbose comments, blank lines - Use initializer list syntax in tests

ravisastryk · 2026-01-07T02:23:22Z

@BillyDonahue Thank you for your thoughtful review and for taking the time. I have addressed the comments. Please re-review and let me know if you have any further suggestions. Also, feel free to commander and make any direct updates so we can collaborate.

BillyDonahue · 2026-01-07T05:47:52Z

@BillyDonahue Thank you for your thoughtful review and for taking the time. I have addressed the comments. Please re-review and let me know if you have any further suggestions. Also, feel free to commander and make any direct updates so we can collaborate.

This looks good. I have put it with modifications into internal review and we should be able to commit soon. Thx.

[SERVER-116125] Defense-in-depth hardening for zlib decompression (CV…

4d3c3c3

…E-2025-14847)

ravisastryk marked this pull request as ready for review January 5, 2026 02:05

BillyDonahue requested changes Jan 5, 2026

View reviewed changes

ravisastryk added 3 commits January 5, 2026 15:54

tests: zlib hardening - remove CVE refs, refactor tests, add helper

0af736e

tests: reduce boilerplate by enhancing helper function

18b1340

- Update assertZlibDecompressBadValue to accept optional output buffer size parameter - Simplify RejectsExcessiveDecompressionRatio test using the enhanced helper - Eliminates code duplication, makes test intent clearer

ravisastryk requested a review from BillyDonahue January 6, 2026 01:07

BillyDonahue requested changes Jan 6, 2026

View reviewed changes

BillyDonahue changed the title ~~[SERVER-116125] Defense-in-depth hardening for zlib decompression (CVE-2025-14847)~~ SERVER-116125 Add zlib decompression validation Jan 6, 2026

ravisastryk requested a review from BillyDonahue January 7, 2026 02:21

		// FIX: Use actual decompressed length for metrics
		// Previous code incorrectly used output.length() (buffer size)

		// Per RFC1950: validate first two bytes (CMF, FLG). CM (CMF & 0x0F) must be
		// Z_DEFLATED (8) and (CMF*256 + FLG) must be a multiple of 31.

		// Maximum allowed decompression ratio to prevent zip bombs
		// A ratio above 1024:1 is suspicious for typical MongoDB messages

		if (output.length() > input.length() * kMaxDecompressionRatio)
		return Status{ErrorCodes::BadValue, "Decompression ratio too large"};

		ASSERT_NOT_OK(result);
		ASSERT_EQ(result.getStatus().code(), ErrorCodes::BadValue);

	assertZlibDecompressBadValue(std::vector<char>{0x78, 0x9c, 0x03, 0x00});
	assertZlibDecompressBadValue({0x78, 0x9c, 0x03, 0x00});

	assertZlibDecompressBadValue(std::vector<char>{});
	assertZlibDecompressBadValue({});

SERVER-116125 Add zlib decompression validation #1628

Are you sure you want to change the base?

SERVER-116125 Add zlib decompression validation #1628

Conversation

ravisastryk commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Testing

References

Uh oh!

ravisastryk commented Jan 5, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BillyDonahue commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ravisastryk commented Jan 6, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BillyDonahue commented Jan 6, 2026

Uh oh!

ravisastryk commented Jan 7, 2026

Uh oh!

BillyDonahue commented Jan 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ravisastryk commented Jan 5, 2026 •

edited

Loading

BillyDonahue commented Jan 5, 2026 •

edited

Loading