PYTHON-5923 Add remaining-buffer bound check in _array_of_documents_to_buffer

[AgentGymLeader]

PYTHON-5923

What

Adds a single upper-bound check on the per-element embedded-document length
(value_length) in _cbson_array_of_documents_to_buffer (in
bson/_cbsonmodule.c), immediately before the pymongo_buffer_write /
memcpy call that copies the element into the output buffer.

if (value_length >= (uint32_t)(size-position)) {
PyObject*InvalidBSON=_error("InvalidBSON");
if (InvalidBSON) {
PyErr_SetString(InvalidBSON, "invalid array content");
Py_DECREF(InvalidBSON);
}
goto fail;
}

Why

_cbson_array_of_documents_to_buffer is the C fast-path used by
Collection.find_raw_batches() and aggregate_raw_batches() to convert a
raw BSON array (from a server firstBatch/nextBatch) into a flat stream
of BSON documents.

The function reads a per-element length value_length from the raw bytes and
then passes it directly to pymongo_buffer_write(buffer, string + position, value_length). Before this patch it only checked the lower bound:

if (value_length<BSON_MIN_SIZE) { … goto fail; }

There was no corresponding upper-bound check that value_length does not
exceed the bytes remaining in the array document (size - position). A
crafted raw-batch reply — from a malicious or compromised server — with an
inner value_length larger than the remaining buffer causes
pymongo_buffer_write → memcpy to read past the end of the heap allocation,
constituting an out-of-bounds read.

The pure-Python counterpart _get_object_size in bson/__init__.py already
enforces this invariant. The C fast-path dropped it.

The loop already guarantees position < size and
(size - position) >= BSON_MIN_SIZE via the prior guard (line ~3221), so the
subtraction size - position cannot underflow (both variables are uint32_t
and position < size is assured).

This belongs to the same bug class as CVE-2024-5629 (out-of-bounds read in
the bson C extension, fixed in 4.6.3) but affects a different function
(_cbson_array_of_documents_to_buffer vs the one patched in 4.6.3).

Change scope

• bson/_cbsonmodule.c: the upper-bound guard block itself (9 lines added).
• test/test_bson.py: two regression tests for the new guard (39 lines added) — one for the oversized-length case, one for the boundary case where the embedded document's declared length exactly consumes the array's own terminator byte.
• doc/changelog.rst: changelog entry for the fix (5 lines added).
• Also includes a whitespace-only ruff-format fix to a slice expression in one of the new tests in test/test_bson.py, to match the file's existing style; no logic changes.

[AgentGymLeader]

Hi team! Could a maintainer please authorize the Evergreen CI patch run when you get a chance? No rush at all — just wanted to flag it so the checks can get started. Thanks so much! 🙏

[NoahStapp]

Hi @AgentGymLeader,

Your new test already passes on master. Can you provide a reproduction script that shows this issue can actually occur?

[AgentGymLeader]

Hi @NoahStapp, thanks for taking a look!

You're right that the new test passes on master — but I believe the issue is that the test verifies the added code path works, while the underlying heap over-read it prevents can still occur on unpatched builds.

Here's a minimal script to illustrate the difference:

importstructimportbson._cbsonasc# BSON array where one element declares value_length=50,# but only 7 bytes remain in the input buffer.outer_size=18data=struct.pack('<I', outer_size) # outer BSON size (4 bytes)data+=b'\x03'# type = embedded documentdata+=b'0\x00'# key "0"data+=struct.pack('<I', 50) # declared sub-doc length = 50 ← lie: only 7 bytes remaindata+=b'\x00'*6# partial content (6 bytes)data+=b'\x00'# outer null terminatortry:
c._array_of_documents_to_buffer(data)
exceptExceptionase:
print(e)
# Without this PR: "bad object or element length"# ↑ caught AFTER pymongo_buffer_write reads 50 bytes from a 7-byte window# With this PR: "invalid array content"# ↑ caught BEFORE the over-read, at the new bounds check

The problem is that pymongo_buffer_write(buffer, string + position, value_length) runs before position += value_length triggers the final position != size - 1 guard. So the heap read already happens — it's just caught downstream rather than at the point of the read.

To confirm the over-read itself: building with -fsanitize=address on a build that does not include this PR should surface an AddressSanitizer heap-buffer-overflow on the crafted input above.

Happy to add an ASan-annotated test or adjust the repro if that would help!

[NoahStapp]

Please adjust the test so that it correctly tests only the fix's new error rather than passing with the existing fallback error on master.

[AgentGymLeader]

@NoahStapp good catch, thanks. Tightened it to assertRaisesRegex(InvalidBSON, "invalid array content") so it keys off the message the new bound check raises. Without the guard that same input ends up on the old fallback path with a different InvalidBSON, so the test fails on master now and only goes green once the check is in. Valid-buffer case is untouched. Force-pushed.

[AgentGymLeader]

@NoahStapp After my latest push, Evergreen is still showing patch must be manually authorized, so the CI run hasn't kicked off.

Could you authorize the patch run when you have a moment? Per your earlier note, the test now checks for the new invalid array content rather than the existing fallback error.

[codeowners-service-app]

Assigned blink1073 for team dbx-python because NoahStapp is out of office.
Assigned sleepyStick for team dbx-python because NoahStapp is out of office.

[AgentGymLeader]

Hi @blink1073, thanks for picking this up. The test tightening NoahStapp asked for is already in — it now asserts on the invalid array content message the new bound check raises, instead of passing through the old fallback path. The one thing still blocking is Evergreen showing patch must be manually authorized, so CI hasn't actually run yet. Could you authorize the patch run when you get a chance? Happy to take another look after that if anything needs adjusting.

[NoahStapp]

Please add a changelog entry calling out this fix, otherwise looks great, thank you!

[AgentGymLeader]

Thanks @NoahStapp! Added a changelog entry calling out the fix.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[Copilot]

Pull request overview

Adds a defensive bounds check to the BSON C-extension fast-path that flattens an array of embedded documents into a contiguous BSON stream, preventing potential out-of-bounds reads on malformed/crafted input.

Changes:

• Add an upper-bound check for per-element embedded document length in bson/_cbsonmodule.c before copying bytes into the output buffer.
• Add a regression test that crafts an embedded document length larger than the remaining bytes and asserts InvalidBSON.
• Document the fix in the 4.18.0 changelog.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
bson/_cbsonmodule.c	Adds an upper-bound guard for value_length to prevent reading past the array buffer.
test/test_bson.py	Adds a regression test covering the oversized embedded-document length case.
doc/changelog.rst	Notes the OOB-read fix in the 4.18.0 changelog.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

[NoahStapp]

Thanks!

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

[NoahStapp]

Thanks!