From e9df748dc4944e3290d84226f865009f7bb07c0c Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sat, 10 Oct 2026 20:07:56 +0200 Subject: [PATCH] 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 --- .../nlohmann/detail/output/binary_writer.hpp | 122 +++++++++++++++++- single_include/nlohmann/json.hpp | 122 +++++++++++++++++- tests/src/unit-large_json.cpp | 63 +++++++++ 3 files changed, 295 insertions(+), 12 deletions(-) diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 212d2658f..55fd8ccb7 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -878,9 +878,9 @@ class binary_writer } } - /// @brief a CBOR or MessagePack array or object whose elements - /// @ref write_cbor_iterative or @ref write_msgpack_iterative is - /// still writing + /// @brief a CBOR, MessagePack, or BON8 array or object whose elements + /// @ref write_cbor_iterative, @ref write_msgpack_iterative, or + /// @ref write_bon8_iterative is still writing struct binary_container_frame { explicit binary_container_frame(const BasicJsonType* value_) noexcept @@ -2570,9 +2570,26 @@ class binary_writer @param[in] j JSON value to serialize @param[in,out] string_open whether the output ends with a non-empty string that has not been terminated with 0xFF + @param[in] depth nesting level of @a j, counted from the + top-level value passed to @ref basic_json::to_bon8 + @throw type_error.316 if a string value or an object key is not valid + UTF-8 + @throw out_of_range.407 if an unsigned integer does not fit int64 + + Values nested deeper than @ref recursion_depth_limit are written by + @ref write_bon8_iterative without the call stack. + + @sa @ref write_cbor + @sa https://github.com/nlohmann/json/issues/5392 */ - void write_bon8_value(const BasicJsonType& j, bool& string_open) + void write_bon8_value(const BasicJsonType& j, bool& string_open, const std::size_t depth = 0) { + if (JSON_HEDLEY_UNLIKELY(depth >= recursion_depth_limit()) && (j.is_array() || j.is_object())) + { + write_bon8_iterative(j, string_open); + return; + } + switch (j.type()) { case value_t::null: @@ -2626,7 +2643,7 @@ class binary_writer for (const auto& el : *j.m_data.m_value.array) { - write_bon8_value(el, string_open); + write_bon8_value(el, string_open, depth + 1); } if (N > 4) @@ -2645,7 +2662,7 @@ class binary_writer for (const auto& el : *j.m_data.m_value.object) { write_bon8_string(el.first, string_open, j); - write_bon8_value(el.second, string_open); + write_bon8_value(el.second, string_open, depth + 1); } if (N > 4) @@ -2682,6 +2699,99 @@ class binary_writer } } + /*! + @brief write @a j with @ref write_bon8_value, or write its marker and push + a frame for @ref write_bon8_iterative to continue with its elements + + A scalar, and an empty array or object, are written out in full and not + pushed. + + @sa @ref write_cbor_value_or_push + */ + void write_bon8_value_or_push(const BasicJsonType& j, bool& string_open, std::vector& stack) + { + if (j.is_array()) + { + const auto N = j.m_data.m_value.array->size(); + write_bon8_marker(static_cast(N <= 4 ? 0x80 + N : 0x85), string_open); + if (N != 0) + { + stack.emplace_back(&j); + } + return; + } + + if (j.is_object()) + { + const auto N = j.m_data.m_value.object->size(); + write_bon8_marker(static_cast(N <= 4 ? 0x86 + N : 0x8B), string_open); + if (N != 0) + { + stack.emplace_back(&j); + } + return; + } + + write_bon8_value(j, string_open); + } + + /*! + @brief write out @a root and everything below it without the call stack + + A container with more than four elements is closed with 0xFE once its + last element is written, as in @ref write_bon8_value. + + @sa @ref write_cbor_iterative + */ + void write_bon8_iterative(const BasicJsonType& root, bool& string_open) + { + // only a container with elements is ever pushed; see write_bon8_value_or_push + std::vector stack; + write_bon8_value_or_push(root, string_open, stack); + + while (!stack.empty()) + { + const binary_container_frame current = stack.back(); + + if (current.value->is_array()) + { + const auto& array = *current.value->m_data.m_value.array; + if (current.array_it == array.cend()) + { + if (array.size() > 4) + { + write_bon8_marker(0xFE, string_open); + } + stack.pop_back(); + continue; + } + + // read the child before pushing: entering it can move every frame + const BasicJsonType* child = &(*current.array_it); + ++stack.back().array_it; + write_bon8_value_or_push(*child, string_open, stack); + } + else + { + const auto& object = *current.value->m_data.m_value.object; + if (current.object_it == object.cend()) + { + if (object.size() > 4) + { + write_bon8_marker(0xFE, string_open); + } + stack.pop_back(); + continue; + } + + write_bon8_string(current.object_it->first, string_open, *current.value); + const BasicJsonType* child = &(current.object_it->second); + ++stack.back().object_it; + write_bon8_value_or_push(*child, string_open, stack); + } + } + } + /*! @brief write a single byte that is not part of a string diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 09f67bdc3..e4cdf09b0 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -22495,9 +22495,9 @@ class binary_writer } } - /// @brief a CBOR or MessagePack array or object whose elements - /// @ref write_cbor_iterative or @ref write_msgpack_iterative is - /// still writing + /// @brief a CBOR, MessagePack, or BON8 array or object whose elements + /// @ref write_cbor_iterative, @ref write_msgpack_iterative, or + /// @ref write_bon8_iterative is still writing struct binary_container_frame { explicit binary_container_frame(const BasicJsonType* value_) noexcept @@ -24187,9 +24187,26 @@ class binary_writer @param[in] j JSON value to serialize @param[in,out] string_open whether the output ends with a non-empty string that has not been terminated with 0xFF + @param[in] depth nesting level of @a j, counted from the + top-level value passed to @ref basic_json::to_bon8 + @throw type_error.316 if a string value or an object key is not valid + UTF-8 + @throw out_of_range.407 if an unsigned integer does not fit int64 + + Values nested deeper than @ref recursion_depth_limit are written by + @ref write_bon8_iterative without the call stack. + + @sa @ref write_cbor + @sa https://github.com/nlohmann/json/issues/5392 */ - void write_bon8_value(const BasicJsonType& j, bool& string_open) + void write_bon8_value(const BasicJsonType& j, bool& string_open, const std::size_t depth = 0) { + if (JSON_HEDLEY_UNLIKELY(depth >= recursion_depth_limit()) && (j.is_array() || j.is_object())) + { + write_bon8_iterative(j, string_open); + return; + } + switch (j.type()) { case value_t::null: @@ -24243,7 +24260,7 @@ class binary_writer for (const auto& el : *j.m_data.m_value.array) { - write_bon8_value(el, string_open); + write_bon8_value(el, string_open, depth + 1); } if (N > 4) @@ -24262,7 +24279,7 @@ class binary_writer for (const auto& el : *j.m_data.m_value.object) { write_bon8_string(el.first, string_open, j); - write_bon8_value(el.second, string_open); + write_bon8_value(el.second, string_open, depth + 1); } if (N > 4) @@ -24299,6 +24316,99 @@ class binary_writer } } + /*! + @brief write @a j with @ref write_bon8_value, or write its marker and push + a frame for @ref write_bon8_iterative to continue with its elements + + A scalar, and an empty array or object, are written out in full and not + pushed. + + @sa @ref write_cbor_value_or_push + */ + void write_bon8_value_or_push(const BasicJsonType& j, bool& string_open, std::vector& stack) + { + if (j.is_array()) + { + const auto N = j.m_data.m_value.array->size(); + write_bon8_marker(static_cast(N <= 4 ? 0x80 + N : 0x85), string_open); + if (N != 0) + { + stack.emplace_back(&j); + } + return; + } + + if (j.is_object()) + { + const auto N = j.m_data.m_value.object->size(); + write_bon8_marker(static_cast(N <= 4 ? 0x86 + N : 0x8B), string_open); + if (N != 0) + { + stack.emplace_back(&j); + } + return; + } + + write_bon8_value(j, string_open); + } + + /*! + @brief write out @a root and everything below it without the call stack + + A container with more than four elements is closed with 0xFE once its + last element is written, as in @ref write_bon8_value. + + @sa @ref write_cbor_iterative + */ + void write_bon8_iterative(const BasicJsonType& root, bool& string_open) + { + // only a container with elements is ever pushed; see write_bon8_value_or_push + std::vector stack; + write_bon8_value_or_push(root, string_open, stack); + + while (!stack.empty()) + { + const binary_container_frame current = stack.back(); + + if (current.value->is_array()) + { + const auto& array = *current.value->m_data.m_value.array; + if (current.array_it == array.cend()) + { + if (array.size() > 4) + { + write_bon8_marker(0xFE, string_open); + } + stack.pop_back(); + continue; + } + + // read the child before pushing: entering it can move every frame + const BasicJsonType* child = &(*current.array_it); + ++stack.back().array_it; + write_bon8_value_or_push(*child, string_open, stack); + } + else + { + const auto& object = *current.value->m_data.m_value.object; + if (current.object_it == object.cend()) + { + if (object.size() > 4) + { + write_bon8_marker(0xFE, string_open); + } + stack.pop_back(); + continue; + } + + write_bon8_string(current.object_it->first, string_open, *current.value); + const BasicJsonType* child = &(current.object_it->second); + ++stack.back().object_it; + write_bon8_value_or_push(*child, string_open, stack); + } + } + } + /*! @brief write a single byte that is not part of a string diff --git a/tests/src/unit-large_json.cpp b/tests/src/unit-large_json.cpp index cd615f4c3..34cf2a166 100644 --- a/tests/src/unit-large_json.cpp +++ b/tests/src/unit-large_json.cpp @@ -400,26 +400,31 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") CHECK(json::from_ubjson(json::to_ubjson(deep_array, true, false)) == deep_array); CHECK(json::from_ubjson(json::to_ubjson(deep_array, true, true)) == deep_array); CHECK(json::from_bjdata(json::to_bjdata(deep_array)) == deep_array); + CHECK(json::from_bon8(json::to_bon8(deep_array)) == deep_array); CHECK(json::from_cbor(json::to_cbor(deep_object)) == deep_object); CHECK(json::from_msgpack(json::to_msgpack(deep_object)) == deep_object); CHECK(json::from_ubjson(json::to_ubjson(deep_object)) == deep_object); CHECK(json::from_ubjson(json::to_ubjson(deep_object, true, true)) == deep_object); CHECK(json::from_bjdata(json::to_bjdata(deep_object)) == deep_object); + CHECK(json::from_bon8(json::to_bon8(deep_object)) == deep_object); CHECK(json::from_cbor(json::to_cbor(empty_array)) == empty_array); CHECK(json::from_msgpack(json::to_msgpack(empty_array)) == empty_array); CHECK(json::from_ubjson(json::to_ubjson(empty_array)) == empty_array); CHECK(json::from_ubjson(json::to_ubjson(empty_array, true, true)) == empty_array); + CHECK(json::from_bon8(json::to_bon8(empty_array)) == empty_array); CHECK(json::from_cbor(json::to_cbor(empty_object)) == empty_object); CHECK(json::from_msgpack(json::to_msgpack(empty_object)) == empty_object); CHECK(json::from_ubjson(json::to_ubjson(empty_object)) == empty_object); + CHECK(json::from_bon8(json::to_bon8(empty_object)) == empty_object); CHECK(json::from_cbor(json::to_cbor(mixed)) == mixed); CHECK(json::from_msgpack(json::to_msgpack(mixed)) == mixed); CHECK(json::from_ubjson(json::to_ubjson(mixed)) == mixed); CHECK(json::from_bjdata(json::to_bjdata(mixed)) == mixed); + CHECK(json::from_bon8(json::to_bon8(mixed)) == mixed); } SECTION("the two ways of writing a value meet at the bound") @@ -432,11 +437,13 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") CHECK(json::from_cbor(json::to_cbor(array)) == array); CHECK(json::from_msgpack(json::to_msgpack(array)) == array); CHECK(json::from_ubjson(json::to_ubjson(array, true, true)) == array); + CHECK(json::from_bon8(json::to_bon8(array)) == array); const json object = nested_object(depth, json(7)); CHECK(json::from_cbor(json::to_cbor(object)) == object); CHECK(json::from_msgpack(json::to_msgpack(object)) == object); CHECK(json::from_bjdata(json::to_bjdata(object)) == object); + CHECK(json::from_bon8(json::to_bon8(object)) == object); } } @@ -481,6 +488,10 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") expected_ubjson.append(depth, ']'); const auto packed_ubjson = json::to_ubjson(array); CHECK(std::string(packed_ubjson.begin(), packed_ubjson.end()) == expected_ubjson); + + std::vector expected_bon8(depth, 0x81); + expected_bon8.push_back(0x90); + CHECK(json::to_bon8(array) == expected_bon8); } } @@ -493,6 +504,7 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") CHECK(json::from_msgpack(json::to_msgpack(object)) == object); CHECK(json::from_ubjson(json::to_ubjson(object, true, true)) == object); CHECK(json::from_bjdata(json::to_bjdata(object)) == object); + CHECK(json::from_bon8(json::to_bon8(object)) == object); const json ndarray = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}}); const json deep_ndarray = nested_array(depth, ndarray); @@ -523,6 +535,27 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") CHECK_THROWS_WITH_AS(json::to_bjdata(deep_discarded), (prefix + "cannot serialize discarded value to BJData").c_str(), json::type_error); } + SECTION("BON8 past the recursion bound: discarded values and errors") + { + const std::size_t depth = nlohmann::detail::recursion_depth_limit() + 50; + + // BON8 writes nothing for a discarded value, deep or not + const json deep_discarded = nested_array(depth, json(json::value_t::discarded)); + CHECK(json::to_bon8(deep_discarded) == std::vector(depth, 0x81)); + + // errors are thrown from below the bound as from above it + const json deep_invalid_string = nested_array(depth, json(std::string("\x80"))); + CHECK_THROWS_AS(json::to_bon8(deep_invalid_string), json::type_error); + + json deep_invalid_key = json::object(); + deep_invalid_key[std::string("\x80")] = 1; + deep_invalid_key = nested_object(depth, deep_invalid_key); + CHECK_THROWS_AS(json::to_bon8(deep_invalid_key), json::type_error); + + const json deep_too_large = nested_array(depth, json(9223372036854775808u)); + CHECK_THROWS_AS(json::to_bon8(deep_too_large), json::out_of_range); + } + SECTION("does not overflow the C++ stack") { const std::size_t depth = 100000; @@ -543,6 +576,27 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") CHECK_NOTHROW(packed = json::to_bjdata(j)); CHECK(json::from_bjdata(packed) == j); + + CHECK_NOTHROW(packed = json::to_bon8(j)); + CHECK(json::from_bon8(packed) == j); + } + + SECTION("BON8 containers with more than four elements past the recursion bound") + { + // such containers are closed with 0xFE, which the iterative writer + // must emit when it leaves them; strings next to each other must + // still be separated with 0xFF + const std::size_t depth = nlohmann::detail::recursion_depth_limit() + 50; + json wide_array = json::array({1, "a", "b", json::array(), 5}); + json wide_object = json({{"k1", "v"}, {"k2", 2}, {"k3", ""}, {"k4", json::object()}, {"k5", nullptr}}); + for (std::size_t i = 0; i < depth; ++i) + { + wide_array = json::array({"s", wide_array, "t", wide_object, true}); + wide_object = json({{"a", wide_object}, {"b", "x"}, {"c", "y"}, {"d", wide_array.size()}, {"e", "z"}}); + } + + CHECK(json::from_bon8(json::to_bon8(wide_array)) == wide_array); + CHECK(json::from_bon8(json::to_bon8(wide_object)) == wide_object); } SECTION("regression test for https://issues.oss-fuzz.com/issues/566583014") @@ -561,5 +615,14 @@ TEST_CASE("issue #5392 - binary writers on deeply nested values") const json j_msgpack = json::from_msgpack(v_msgpack); CHECK(json::to_msgpack(j_msgpack) == v_msgpack); } + + SECTION("regression test for https://issues.oss-fuzz.com/issues/572238015") + { + // the BON8 analogue: 200000 nested one-element arrays down to null + std::vector v(200000, 0x81); + v.push_back(0xFA); + const json j = json::from_bon8(v); + CHECK(json::to_bon8(j) == v); + } }