diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index f699de1b9..1cc3e10c9 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -130,6 +130,7 @@ class binary_reader const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) { sax = sax_; + container_stack.clear(); bool result = false; switch (format) @@ -179,6 +180,57 @@ class binary_reader } private: + //////////////////////// + // nested containers // + //////////////////////// + + /*! + @brief a container that has been opened and not closed yet + + The binary readers do not call themselves once per nesting level. Like + @ref parser::sax_parse_internal, which does the same for JSON text, they + keep the containers they are inside of on a heap-allocated stack, so that + the native call stack does not grow with the nesting depth of the input + and a deeply nested value is bounded by memory rather than by the stack + (see #5104). + + The members are ordered widest first: frames are stored in a vector, and + declaring the `bool` first would pad the struct out for no reason. + */ + struct container_frame + { + /// number of elements that have not been read yet + std::size_t remaining = 0; + /// whether to close this container with end_object() or end_array() + bool is_object = false; + }; + + /*! + @brief open a nested array or object + + Emits the SAX start event and records the container. This is the only + place the binary readers start a container, so a check that rejects one + can be made here and is then guaranteed to run before the start event. + + @param[in] is_object whether an object (true) or an array (false) begins + @param[in] len number of elements the container declares + + @return whether the SAX parser accepted the start event + */ + bool enter_container(const bool is_object, const std::size_t len) + { + if (JSON_HEDLEY_UNLIKELY(is_object ? !sax->start_object(len) : !sax->start_array(len))) + { + return false; + } + + container_frame frame; + frame.remaining = len; + frame.is_object = is_object; + container_stack.push_back(frame); + return true; + } + ////////// // BSON // ////////// @@ -1422,7 +1474,17 @@ class binary_reader /*! @return whether a valid MessagePack value was passed to the SAX parser */ - bool parse_msgpack_internal() + /*! + @brief read one MessagePack value + + Reads a single value and passes it to the SAX parser. A value that begins + a container is not read to its end: the container is opened with + @ref enter_container and its elements are read by + @ref parse_msgpack_internal, so that nesting does not consume native stack. + + @return whether reading the value succeeded + */ + bool parse_msgpack_value() { switch (get()) { @@ -1578,7 +1640,7 @@ class binary_reader case 0x8D: case 0x8E: case 0x8F: - return get_msgpack_object(conditional_static_cast(static_cast(current) & 0x0Fu)); + return enter_container(/*is_object*/true, conditional_static_cast(static_cast(current) & 0x0Fu)); // fixarray case 0x90: @@ -1597,7 +1659,7 @@ class binary_reader case 0x9D: case 0x9E: case 0x9F: - return get_msgpack_array(conditional_static_cast(static_cast(current) & 0x0Fu)); + return enter_container(/*is_object*/false, conditional_static_cast(static_cast(current) & 0x0Fu)); // fixstr case 0xA0: @@ -1728,25 +1790,25 @@ class binary_reader case 0xDC: // array 16 { std::uint16_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_array(static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/false, static_cast(len)); } case 0xDD: // array 32 { std::uint32_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_array(conditional_static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/false, conditional_static_cast(len)); } case 0xDE: // map 16 { std::uint16_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_object(static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/true, static_cast(len)); } case 0xDF: // map 32 { std::uint32_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_object(conditional_static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/true, conditional_static_cast(len)); } // negative fixint @@ -1994,55 +2056,69 @@ class binary_reader } /*! - @param[in] len the length of the array - @return whether array creation completed + @brief read a MessagePack value and everything nested inside it + + Reads values until the one that was begun here is complete, resuming the + enclosing container each time an element ends, so that the nesting depth + of the input costs heap rather than native stack (see #5104). + + @return whether reading the value succeeded */ - bool get_msgpack_array(const std::size_t len) + bool parse_msgpack_internal() { - if (JSON_HEDLEY_UNLIKELY(!sax->start_array(len))) - { - return false; - } - - for (std::size_t i = 0; i < len; ++i) - { - if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_internal())) - { - return false; - } - } - - return sax->end_array(); - } - - /*! - @param[in] len the length of the object - @return whether object creation completed - */ - bool get_msgpack_object(const std::size_t len) - { - if (JSON_HEDLEY_UNLIKELY(!sax->start_object(len))) - { - return false; - } - + // the key currently being read; hoisted out of the loop so that its + // capacity is reused across elements and across nesting levels string_t key; - for (std::size_t i = 0; i < len; ++i) + + while (true) { - get(); - if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + if (!container_stack.empty()) + { + // copied out before anything can push onto the stack and + // invalidate a reference into it + const bool is_object = container_stack.back().is_object; + + if (container_stack.back().remaining == 0) + { + container_stack.pop_back(); + if (JSON_HEDLEY_UNLIKELY(is_object ? !sax->end_object() : !sax->end_array())) + { + return false; + } + // the value begun here is complete once its container is + if (container_stack.empty()) + { + return true; + } + continue; + } + + // claim the element about to be read + --container_stack.back().remaining; + + if (is_object) + { + get(); + key.clear(); + if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + { + return false; + } + } + } + + if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_value())) { return false; } - if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_internal())) + // a value that opened a container left it on the stack; one that + // did not, and that was not inside a container, was the whole value + if (container_stack.empty()) { - return false; + return true; } - key.clear(); } - - return sax->end_object(); } //////////// @@ -3347,6 +3423,9 @@ class binary_reader /// the SAX parser json_sax_t* sax = nullptr; + /// the containers that have been opened and not closed yet; see @ref container_frame + std::vector container_stack{}; + // excluded markers in bjdata optimized type #define JSON_BINARY_READER_MAKE_BJD_OPTIMIZED_TYPE_MARKERS_ \ make_array('F', 'H', 'N', 'S', 'T', 'Z', '[', '{') diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 7b99e4591..1a9a9a092 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -10817,6 +10817,7 @@ class binary_reader const cbor_tag_handler_t tag_handler = cbor_tag_handler_t::error) { sax = sax_; + container_stack.clear(); bool result = false; switch (format) @@ -10866,6 +10867,57 @@ class binary_reader } private: + //////////////////////// + // nested containers // + //////////////////////// + + /*! + @brief a container that has been opened and not closed yet + + The binary readers do not call themselves once per nesting level. Like + @ref parser::sax_parse_internal, which does the same for JSON text, they + keep the containers they are inside of on a heap-allocated stack, so that + the native call stack does not grow with the nesting depth of the input + and a deeply nested value is bounded by memory rather than by the stack + (see #5104). + + The members are ordered widest first: frames are stored in a vector, and + declaring the `bool` first would pad the struct out for no reason. + */ + struct container_frame + { + /// number of elements that have not been read yet + std::size_t remaining = 0; + /// whether to close this container with end_object() or end_array() + bool is_object = false; + }; + + /*! + @brief open a nested array or object + + Emits the SAX start event and records the container. This is the only + place the binary readers start a container, so a check that rejects one + can be made here and is then guaranteed to run before the start event. + + @param[in] is_object whether an object (true) or an array (false) begins + @param[in] len number of elements the container declares + + @return whether the SAX parser accepted the start event + */ + bool enter_container(const bool is_object, const std::size_t len) + { + if (JSON_HEDLEY_UNLIKELY(is_object ? !sax->start_object(len) : !sax->start_array(len))) + { + return false; + } + + container_frame frame; + frame.remaining = len; + frame.is_object = is_object; + container_stack.push_back(frame); + return true; + } + ////////// // BSON // ////////// @@ -12109,7 +12161,17 @@ class binary_reader /*! @return whether a valid MessagePack value was passed to the SAX parser */ - bool parse_msgpack_internal() + /*! + @brief read one MessagePack value + + Reads a single value and passes it to the SAX parser. A value that begins + a container is not read to its end: the container is opened with + @ref enter_container and its elements are read by + @ref parse_msgpack_internal, so that nesting does not consume native stack. + + @return whether reading the value succeeded + */ + bool parse_msgpack_value() { switch (get()) { @@ -12265,7 +12327,7 @@ class binary_reader case 0x8D: case 0x8E: case 0x8F: - return get_msgpack_object(conditional_static_cast(static_cast(current) & 0x0Fu)); + return enter_container(/*is_object*/true, conditional_static_cast(static_cast(current) & 0x0Fu)); // fixarray case 0x90: @@ -12284,7 +12346,7 @@ class binary_reader case 0x9D: case 0x9E: case 0x9F: - return get_msgpack_array(conditional_static_cast(static_cast(current) & 0x0Fu)); + return enter_container(/*is_object*/false, conditional_static_cast(static_cast(current) & 0x0Fu)); // fixstr case 0xA0: @@ -12415,25 +12477,25 @@ class binary_reader case 0xDC: // array 16 { std::uint16_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_array(static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/false, static_cast(len)); } case 0xDD: // array 32 { std::uint32_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_array(conditional_static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/false, conditional_static_cast(len)); } case 0xDE: // map 16 { std::uint16_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_object(static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/true, static_cast(len)); } case 0xDF: // map 32 { std::uint32_t len{}; - return get_number(input_format_t::msgpack, len) && get_msgpack_object(conditional_static_cast(len)); + return get_number(input_format_t::msgpack, len) && enter_container(/*is_object*/true, conditional_static_cast(len)); } // negative fixint @@ -12681,55 +12743,69 @@ class binary_reader } /*! - @param[in] len the length of the array - @return whether array creation completed + @brief read a MessagePack value and everything nested inside it + + Reads values until the one that was begun here is complete, resuming the + enclosing container each time an element ends, so that the nesting depth + of the input costs heap rather than native stack (see #5104). + + @return whether reading the value succeeded */ - bool get_msgpack_array(const std::size_t len) + bool parse_msgpack_internal() { - if (JSON_HEDLEY_UNLIKELY(!sax->start_array(len))) - { - return false; - } - - for (std::size_t i = 0; i < len; ++i) - { - if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_internal())) - { - return false; - } - } - - return sax->end_array(); - } - - /*! - @param[in] len the length of the object - @return whether object creation completed - */ - bool get_msgpack_object(const std::size_t len) - { - if (JSON_HEDLEY_UNLIKELY(!sax->start_object(len))) - { - return false; - } - + // the key currently being read; hoisted out of the loop so that its + // capacity is reused across elements and across nesting levels string_t key; - for (std::size_t i = 0; i < len; ++i) + + while (true) { - get(); - if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + if (!container_stack.empty()) + { + // copied out before anything can push onto the stack and + // invalidate a reference into it + const bool is_object = container_stack.back().is_object; + + if (container_stack.back().remaining == 0) + { + container_stack.pop_back(); + if (JSON_HEDLEY_UNLIKELY(is_object ? !sax->end_object() : !sax->end_array())) + { + return false; + } + // the value begun here is complete once its container is + if (container_stack.empty()) + { + return true; + } + continue; + } + + // claim the element about to be read + --container_stack.back().remaining; + + if (is_object) + { + get(); + key.clear(); + if (JSON_HEDLEY_UNLIKELY(!get_msgpack_string(key) || !sax->key(key))) + { + return false; + } + } + } + + if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_value())) { return false; } - if (JSON_HEDLEY_UNLIKELY(!parse_msgpack_internal())) + // a value that opened a container left it on the stack; one that + // did not, and that was not inside a container, was the whole value + if (container_stack.empty()) { - return false; + return true; } - key.clear(); } - - return sax->end_object(); } //////////// @@ -14034,6 +14110,9 @@ class binary_reader /// the SAX parser json_sax_t* sax = nullptr; + /// the containers that have been opened and not closed yet; see @ref container_frame + std::vector container_stack{}; + // excluded markers in bjdata optimized type #define JSON_BINARY_READER_MAKE_BJD_OPTIMIZED_TYPE_MARKERS_ \ make_array('F', 'H', 'N', 'S', 'T', 'Z', '[', '{') diff --git a/tests/src/unit-msgpack.cpp b/tests/src/unit-msgpack.cpp index 4358cd464..75c2ae464 100644 --- a/tests/src/unit-msgpack.cpp +++ b/tests/src/unit-msgpack.cpp @@ -1598,6 +1598,67 @@ TEST_CASE("MessagePack") } // use this testcase outside [hide] to run it with Valgrind +TEST_CASE("MessagePack nesting does not consume the call stack") +{ + // Reading a container used to call back into the value reader once per + // element, so the native call stack grew with the nesting depth of the + // input: one frame per byte for repeated 0x91 (a one-element array), which + // crashes the process long before the input is exhausted (#5104). The + // containers are kept on a heap stack now. + // + // Note that deeply nested values must not be compared, copied or dumped + // here: those operations are still recursive, and would reintroduce the + // very crash this checks for. Depth is measured by descending instead. + + SECTION("an unterminated chain is reported, not crashed on") + { + json _; + const std::vector input(300000, 0x91); + CHECK_THROWS_WITH_AS(_ = json::from_msgpack(input), "[json.exception.parse_error.110] parse error at byte 300001: syntax error while parsing MessagePack value: unexpected end of input", json::parse_error&); + CHECK(json::from_msgpack(input, true, false).is_discarded()); + } + + SECTION("a well-formed deep value is read through the SAX interface") + { + std::vector input(300000, 0x91); + input.push_back(0x01); // innermost value + + SaxCountdown accept_all(600001); + CHECK(json::sax_parse(input, &accept_all, json::input_format_t::msgpack)); + } + + SECTION("a well-formed deep value is read into a value") + { + const std::size_t depth = 10000; + std::vector input(depth, 0x91); + input.push_back(0x01); + + json j = json::from_msgpack(input); + + std::size_t measured = 0; + const json* p = &j; + while (p->is_array() && !p->empty()) + { + p = &p->front(); + ++measured; + } + CHECK(measured == depth); + CHECK(p->is_number()); + } + + SECTION("containers are still read the same way") + { + CHECK(json::from_msgpack(std::vector({0x90})) == json::array()); + CHECK(json::from_msgpack(std::vector({0x80})) == json::object()); + CHECK(json::from_msgpack(std::vector({0x92, 0x90, 0x80})) == json({json::array(), json::object()})); + CHECK(json::from_msgpack(std::vector({0x91, 0x91, 0x91, 0x90})) == json({{{json::array()}}})); + CHECK(json::from_msgpack(std::vector({0x81, 0xA1, 'a', 0x81, 0xA1, 'b', 0x92, 0x01, 0x02})) == json({{"a", {{"b", {1, 2}}}}})); + // array 16 and map 32, i.e. the counted forms + CHECK(json::from_msgpack(std::vector({0xDC, 0x00, 0x02, 0x01, 0x02})) == json({1, 2})); + CHECK(json::from_msgpack(std::vector({0xDF, 0x00, 0x00, 0x00, 0x01, 0xA1, 'k', 0xC3})) == json({{"k", true}})); + } +} + TEST_CASE("single MessagePack roundtrip") { SECTION("sample.json")