Reject MessagePack/BSON binary subtypes that don't fit their wire format - #5469
Open
nlohmann wants to merge 2 commits into
Open
Reject MessagePack/BSON binary subtypes that don't fit their wire format#5469nlohmann wants to merge 2 commits into
nlohmann wants to merge 2 commits into
Conversation
nlohmann
changed the base branch from
develop
to
fix/insert-array-iterator-check
September 5, 2026 14:53
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 14:57
1331cd0 to
a1a69a8
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 15:07
a1a69a8 to
2e83234
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 16:27
2e83234 to
7705208
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 16:43
7705208 to
51ff50b
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 18:00
51ff50b to
572c308
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 5, 2026 19:59
572c308 to
93a1a8a
Compare
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
2 times, most recently
from
September 6, 2026 11:26
bda5371 to
6a81cf3
Compare
gregmarr
reviewed
Sep 8, 2026
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
3 times, most recently
from
September 8, 2026 18:11
6a81cf3 to
795b11f
Compare
gregmarr
approved these changes
Sep 8, 2026
Both formats store byte_container_with_subtype's subtype (a uint64_t) in a single byte. The writers cast to std::int8_t/std::uint8_t without a range check, so subtypes above 255 were silently truncated modulo 256 instead of raising an error. Throw out_of_range.413 instead when the subtype exceeds the representable range of 0-255. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
unit-regression2.cpp is already at the edge of what the MinGW linker can relocate; adding this test's ~26 lines tips test-regression2_cpp20 (clang, Windows) over into "relocation truncated to fit: IMAGE_REL_AMD64_REL32 against `.rdata'" (see 8ce64b9 / b82717c for the same failure mode). Split the test along format lines instead: MessagePack assertions move to unit-msgpack.cpp, BSON assertions to unit-bson.cpp. The CBOR round-trip guard is dropped as redundant -- unit-cbor.cpp's "Tagged values" section already round-trips subtypes up to 8589934590, far past the 70000 checked here. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
fix/msgpack-bson-subtype-range
branch
from
September 9, 2026 11:17
795b11f to
1b2b595
Compare
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.
Summary
byte_container_with_subtype::subtype()is astd::uint64_t, but the MessagePack ext-type byte and the BSON binary subtype byte are each a single byte on the wire. Both writers (include/nlohmann/detail/output/binary_writer.hpp) cast the subtype down to a single byte without any range check, silently wrapping modulo 256 instead of erroring — a silent data-corruption bug:The valid range for both formats is actually the full 0–255 (a full unsigned byte), not just 0–127 — the MessagePack reader already round-trips 128–255 correctly via a signed/unsigned reinterpretation of the ext-type byte, and the BSON reader reads the subtype directly as
uint8_t. Only subtypes ≥ 256 are affected. The CBOR writer already handles the full 64-bit range correctly (choosing 1/2/4/8-byte tag encodings), and the BSON writer already throwsout_of_range.407-adjacent errors for other values it can't represent, so a check here is consistent with existing behavior.Fix
Add a range check (
subtype() > 255) before each of the two truncating casts, throwing a newout_of_range.413(the next free out_of_range code after the existing.412) instead of silently truncating. A binary value with no subtype at all is unaffected.Test plan
tests/src/unit-regression2.cppcoverage: MessagePack/BSON subtype 0/200/255 still round-trip correctly (regression guard for the now-confirmed-correct 128–255 range); subtype 256/70000/300 now throwout_of_range.413with the exact expected message; CBOR (already correct) remains unaffected at subtype 70000; a binary value with no subtype at all is unaffected.docs/mkdocs/docs/home/exceptions.md(json.exception.out_of_range.413), matching the existing style.single_include/nlohmann/json.hppviamake amalgamate.Breaking change?
No breaking changes to the public API — this adds a new exception code (
out_of_range.413) and only affects inputs that previously silently corrupted data (a subtype ≥ 256 being written and read back as a different, wrong value). No valid/previously-correct round-trip changes behavior; serializing a binary value whose subtype is ≥ 256 now throws instead of silently producing a wrong result.🤖 Generated with Claude Code