Repository navigation
Stop binary writers overflowing the stack on deep values - #5781
Merged
Merged
Conversation
5 tasks done
to_cbor, to_msgpack, and to_ubjson recurse once per nesting level. The parser is iterative, so a value the library accepts can crash on the way back out. Keep the existing recursive path for the first 128 levels and finish anything deeper on a heap stack. Output is unchanged. BSON is left alone because its extra size walk is a separate change. Rebased onto the value-type output sink. The heap frames now initialize every member, which is what -Weffc++ was rejecting. See #5392. Signed-off-by: ayush-singh-0601 <singhayush062006@gmail.com> (cherry picked from commit cf65ac4) Signed-off-by: Niels Lohmann <mail@nlohmann.me>
… limit Address the open review on the non-recursive CBOR/MessagePack/UBJSON/BJData writers (#5518): - Delete the CBOR array/object prefix helpers; both the recursive and iterative paths call write_cbor_head(), which already existed on develop. - MessagePack: share one write_msgpack_array_prefix()/write_msgpack_object_prefix() helper per container kind between the recursive and iterative paths, both going through to_msgpack_length() so an over-long container throws out_of_range.412 identically either way. - Reuse detail::recursion_depth_limit() instead of a separate constant, the same bound serializer::dump() and write_bson_document() already use. - Redesign the frames after bson_frame/dump_frame: only a container with elements is ever pushed, its header is written at the point it is pushed, and the iterator is set in the frame's constructor instead of a default-then-assign two-step with a since-removed "started" flag. The UBJSON frame keeps only the value pointer, the per-element prefix_required flag, and the iterator; write_closer and is_object are no longer stored, since the former is always !use_count (use_count is constant for the whole document) and the latter follows from value->is_object(). - Factor the BJData ND-array shape check into is_bjdata_ndarray(), used by both the recursive object case and the iterative pushing logic. - Give the frame classes the GCC -Weffc++ treatment already used for diff_frame: a noexcept converting constructor plus the five special members defaulted with no explicit noexcept. - Fix two @ref self-references in write_cbor/write_msgpack/write_ubjson's own doc comments to point at the public to_cbor/to_msgpack/to_ubjson/to_bjdata API instead. - The iterative object-key write for CBOR/MessagePack now runs the same strict-mode check_utf8() against the parent object as diagnostics context that the recursive path already ran, so the two paths raise identical diagnostics across the switch-over. - Rewrite the tests: round trips instead of a bare size check, byte-exact comparisons against the recursive output at depths around the bound, a deep object and a BJData ND-array past the bound, a deep discarded value (type_error.321), and the OSS-Fuzz 566583014 CBOR/MessagePack regression. BSON is unaffected by this change; it already walks its documents iteratively and is covered separately by #5553. Co-authored-by: ayush-singh-0601 <179524189+ayush-singh-0601@users.noreply.github.com> Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
force-pushed
the
binary-writer-iterative
branch
from
October 7, 2026 14:44
b8d7d56 to
3f8fee0
Compare
gregmarr
approved these changes
Oct 7, 2026
nlohmann
added a commit
that referenced
this pull request
Oct 7, 2026
Resolve the conflicts with #5781 (iterative binary writers) and the BJData ND-array change by taking develop's code, then name its exception ids and use invalid_byte() for the moved _ArrayType_ check. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
3 tasks done
nlohmann
added a commit
that referenced
this pull request
Oct 8, 2026
Two test problems from #5781 broke CI on develop: CAPTURE(depth); trips clang's -Wextra-semi-stmt, and the type_error.321 messages did not account for the diagnostics path prefix. Signed-off-by: Niels Lohmann <mail@nlohmann.me>
nlohmann
added a commit
that referenced
this pull request
Oct 8, 2026
* Use the with_*_t aliases in tests, examples, and docs Replace spelled-out basic_json<...> instantiations that only change one or two template parameters with nlohmann::json::with_*_t (or ordered_json::with_*_t when the object type is ordered_map). Types that change all three number types chain with_integers_t and with_float_t. The raw basic_json<...> spelling stays where the template parameter list itself is the subject: the alias tests in unit-udt.cpp, explicit instantiations, and the ordered_json/compile-time docs. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Fix unit-large_json for clang and JSON_DIAGNOSTICS Two test problems from #5781 broke CI on develop: CAPTURE(depth); trips clang's -Wextra-semi-stmt, and the type_error.321 messages did not account for the diagnostics path prefix. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Use static_cast in unit-hash for clang-tidy #5772 added functional casts that clang-tidy reports as C-style casts (google-readability-casting). Also append a char instead of a one-character string in unit-large_json. Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Declare the expected message prefix const in unit-large_json Without JSON_DIAGNOSTICS the prefix was never modified, which clang-tidy reports (misc-const-correctness). Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>
This was referenced Oct 9, 2026
nlohmann
added a commit
that referenced
this pull request
Oct 11, 2026
* Stop the BON8 writer overflowing the stack on deep values to_bon8() recursed once per nesting level, so a value the iterative BON8 reader accepts (e.g. ~24k nested one-element arrays) crashed on the way back out. #5781 bounded the CBOR, MessagePack, and UBJSON/BJData writers, but BON8 was merged before it and was not covered. Apply the same scheme: recurse for the first recursion_depth_limit() levels, then finish the value with write_bon8_iterative, which keeps the open containers on a heap stack (reusing binary_container_frame) and writes the 0xFE closer when it leaves a container with more than four elements. The output is byte-for-byte unchanged. Fixes https://issues.oss-fuzz.com/issues/572238015 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Merge the array and object branches of the BON8 iterative writer write_bon8_iterative and write_bon8_value_or_push handled arrays and objects in separate branches that repeated the end-of-container check, the 0xFE closer and the marker computation. Share those parts and branch only where arrays and objects really differ. The output is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Hz7VJi1FTKr6gpseLErbbS Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
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.
Takes over #5518 by @ayush-singh-0601 (thank you!), rebased on
developand with the open review comments addressed.to_cbor,to_msgpack,to_ubjsonandto_bjdatarecursed once per nesting level. Parsing doesn't recurse, so a value the library happily builds could still overflow the stack when serialized. This is the crash in OSS-Fuzz issue 566583014 (parse_cbor_fuzzer, stack overflow inbinary_writer::write_cbor).Each writer now uses the same bounded descent as
dump(): it recurses as before up todetail::recursion_depth_limit()levels and writes anything deeper with an explicit heap stack. The output is the same bytes as before.Commits
developresolved (write_cbor_headfrom Remove unreachable branches from the binary writer #5583,to_msgpack_lengthfrom Throw instead of writing MessagePack lengths beyond UINT32_MAX #5584,is_bjdata_excluded_type_markerfrom Stop allocating the BJData excluded-marker list per container #5555). The author's second commit 7301b06 isn't included because it only changed CBOR prefix helpers thatwrite_cbor_headreplaces.detail::recursion_depth_limit()instead of a new limit, andwrite_cbor_headinstead of the CBOR prefix helpers.to_msgpack_length(out_of_range.412aboveUINT32_MAX).bson_frame. There is nostartedflag. The UBJSON frame stores only the value,prefix_requiredand the iterator.is_bjdata_ndarray()is shared by the recursive path and the iterative path.type_error.321on both paths. Strict UTF-8 key checks report the same diagnostics context on both paths.to_*functions instead of to themselves.BSON isn't in this PR. Its writer stopped recursing in #5553.
Verification
developon randomjsonandordered_jsonvalues nested 120–140 levels deep, for CBOR, MessagePack, UBJSON and BJData in everyuse_size/use_typecombination.unit-large_json.cpppasses in C++11 and C++17 on a 1 MiB stack. No-Weverythingwarnings inbinary_writer.hpp.Public API
No breaking changes. Serialization output is unchanged. Values nested deeper than the recursion limit no longer crash.
CI on this PR is expected to fail until #5779 fixes
develop.Closes #5392
Closes #5518
🤖 Generated with Claude Code