From 25a4333a31e7ab80973c4e392afb58f3b0ce707d Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 6 Sep 2026 16:46:29 +0200 Subject: [PATCH] Stop CBOR indefinite-length strings from recursing per chunk get_cbor_string() and get_cbor_binary() handled the indefinite-length forms (0x7F and 0x5F) by calling themselves once per chunk. Each chunk therefore cost a native stack frame, and since a chunk may itself be an indefinite- length string, an input of repeated 0x7F bytes reached one frame per input byte: 200,000 of them crash the process with SIGSEGV before a single byte is rejected. This is the same defect as #5104, in a path the container-level work does not touch. Count the open levels instead of recursing through them. That is enough here because every chunk is appended to the same result -- get_bytes() writes at result.size() -- so there is no per-level state to keep. The temporary chunk string and its copy into the result go away with the recursion. The definite-length cases move to get_cbor_string_chunk() and get_cbor_binary_chunk() unchanged, including their error messages, which still name 0x7F and 0x5F because those are handled one level up. Behaviour is unchanged. Comparing against develop over the interesting byte sequences -- empty, single-chunk, nested, over-closed and truncated forms, both strings and byte arrays, and an indefinite-length map key -- produces identical values, error codes, messages and byte offsets. The 200,000-level input now reports parse_error.110 at byte 200001 instead of crashing. Note that nesting these is not valid CBOR: RFC 8949, Section 3.2.3 forbids it. This does not change that either way -- it has always been accepted, and rejecting it is a separate decision (#5317, #5325). Should it be rejected later, that is now one condition on the level counter rather than a change to the control flow. Signed-off-by: Niels Lohmann --- .../nlohmann/detail/input/binary_reader.hpp | 188 +++++++++++++----- single_include/nlohmann/json.hpp | 188 +++++++++++++----- tests/src/unit-cbor.cpp | 52 +++++ 3 files changed, 326 insertions(+), 102 deletions(-) diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 557d7669c..1e05c08f3 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -996,23 +996,21 @@ class binary_reader } /*! - @brief reads a CBOR string + @brief reads a definite-length CBOR string - This function first reads starting bytes to determine the expected - string length and then copies this number of bytes into a string. - Additionally, CBOR's strings with indefinite lengths are supported. + Reads everything @ref get_cbor_string accepts except the indefinite-length + form, which that function handles itself. The bytes are appended to @a + result, so consecutive chunks of an indefinite-length string can be read + into the same string. - @param[out] result created string + @param[out] result string the bytes are appended to @return whether string creation completed - */ - bool get_cbor_string(string_t& result) - { - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "string"))) - { - return false; - } + @pre @a current is not EOF + */ + bool get_cbor_string_chunk(string_t& result) + { switch (current) { // UTF-8 string (0x00..0x17 bytes follow) @@ -1068,20 +1066,6 @@ class binary_reader return get_number(input_format_t::cbor, len) && get_string(input_format_t::cbor, len, result); } - case 0x7F: // UTF-8 string (indefinite length) - { - while (get() != 0xFF) - { - string_t chunk; - if (!get_cbor_string(chunk)) - { - return false; - } - result.append(chunk); - } - return true; - } - default: { auto last_token = get_token_string(); @@ -1092,23 +1076,82 @@ class binary_reader } /*! - @brief reads a CBOR byte array + @brief reads a CBOR string This function first reads starting bytes to determine the expected - byte array length and then copies this number of bytes into the byte array. - Additionally, CBOR's byte arrays with indefinite lengths are supported. + string length and then copies this number of bytes into a string. + Additionally, CBOR's strings with indefinite lengths are supported. - @param[out] result created byte array + @param[out] result created string + + @return whether string creation completed + */ + bool get_cbor_string(string_t& result) + { + // number of indefinite-length strings that have been opened and not + // closed yet. RFC 8949, Section 3.2.3 does not permit nesting them, + // but this reader has always accepted it, so the open levels are + // counted instead of recursed through, which overflowed the stack for + // an input of repeated 0x7F bytes (see #5104). Every chunk is appended + // to the same result, so no per-level state is needed. + std::size_t open = 0; + + while (true) + { + if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "string"))) + { + return false; + } + + if (current == 0x7F) // UTF-8 string (indefinite length) + { + ++open; + get(); + continue; + } + + // a break marker closes the innermost indefinite-length string; + // outside of one it is not a string and falls through to the error + if (open != 0 && current == 0xFF) + { + if (--open == 0) + { + return true; + } + get(); + continue; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result))) + { + return false; + } + + if (open == 0) + { + return true; + } + + get(); + } + } + + /*! + @brief reads a definite-length CBOR byte array + + Reads everything @ref get_cbor_binary accepts except the indefinite-length + form, which that function handles itself. The bytes are appended to @a + result, so consecutive chunks of an indefinite-length byte array can be + read into the same byte array. + + @param[out] result byte array the bytes are appended to @return whether byte array creation completed - */ - bool get_cbor_binary(binary_t& result) - { - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "binary"))) - { - return false; - } + @pre @a current is not EOF + */ + bool get_cbor_binary_chunk(binary_t& result) + { switch (current) { // Binary data (0x00..0x17 bytes follow) @@ -1168,20 +1211,6 @@ class binary_reader get_binary(input_format_t::cbor, len, result); } - case 0x5F: // Binary data (indefinite length) - { - while (get() != 0xFF) - { - binary_t chunk; - if (!get_cbor_binary(chunk)) - { - return false; - } - result.insert(result.end(), chunk.begin(), chunk.end()); - } - return true; - } - default: { auto last_token = get_token_string(); @@ -1191,6 +1220,63 @@ class binary_reader } } + /*! + @brief reads a CBOR byte array + + This function first reads starting bytes to determine the expected + byte array length and then copies this number of bytes into the byte array. + Additionally, CBOR's byte arrays with indefinite lengths are supported. + + @param[out] result created byte array + + @return whether byte array creation completed + */ + bool get_cbor_binary(binary_t& result) + { + // the open indefinite-length byte arrays are counted rather than + // recursed through, for the reason given in @ref get_cbor_string + std::size_t open = 0; + + while (true) + { + if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "binary"))) + { + return false; + } + + if (current == 0x5F) // Binary data (indefinite length) + { + ++open; + get(); + continue; + } + + // a break marker closes the innermost indefinite-length byte + // array; outside of one it falls through to the error below + if (open != 0 && current == 0xFF) + { + if (--open == 0) + { + return true; + } + get(); + continue; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result))) + { + return false; + } + + if (open == 0) + { + return true; + } + + get(); + } + } + /*! @brief narrow a definite CBOR array/map length to std::size_t diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 27e50d581..f2122e3ee 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -11683,23 +11683,21 @@ class binary_reader } /*! - @brief reads a CBOR string + @brief reads a definite-length CBOR string - This function first reads starting bytes to determine the expected - string length and then copies this number of bytes into a string. - Additionally, CBOR's strings with indefinite lengths are supported. + Reads everything @ref get_cbor_string accepts except the indefinite-length + form, which that function handles itself. The bytes are appended to @a + result, so consecutive chunks of an indefinite-length string can be read + into the same string. - @param[out] result created string + @param[out] result string the bytes are appended to @return whether string creation completed - */ - bool get_cbor_string(string_t& result) - { - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "string"))) - { - return false; - } + @pre @a current is not EOF + */ + bool get_cbor_string_chunk(string_t& result) + { switch (current) { // UTF-8 string (0x00..0x17 bytes follow) @@ -11755,20 +11753,6 @@ class binary_reader return get_number(input_format_t::cbor, len) && get_string(input_format_t::cbor, len, result); } - case 0x7F: // UTF-8 string (indefinite length) - { - while (get() != 0xFF) - { - string_t chunk; - if (!get_cbor_string(chunk)) - { - return false; - } - result.append(chunk); - } - return true; - } - default: { auto last_token = get_token_string(); @@ -11779,23 +11763,82 @@ class binary_reader } /*! - @brief reads a CBOR byte array + @brief reads a CBOR string This function first reads starting bytes to determine the expected - byte array length and then copies this number of bytes into the byte array. - Additionally, CBOR's byte arrays with indefinite lengths are supported. + string length and then copies this number of bytes into a string. + Additionally, CBOR's strings with indefinite lengths are supported. - @param[out] result created byte array + @param[out] result created string + + @return whether string creation completed + */ + bool get_cbor_string(string_t& result) + { + // number of indefinite-length strings that have been opened and not + // closed yet. RFC 8949, Section 3.2.3 does not permit nesting them, + // but this reader has always accepted it, so the open levels are + // counted instead of recursed through, which overflowed the stack for + // an input of repeated 0x7F bytes (see #5104). Every chunk is appended + // to the same result, so no per-level state is needed. + std::size_t open = 0; + + while (true) + { + if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "string"))) + { + return false; + } + + if (current == 0x7F) // UTF-8 string (indefinite length) + { + ++open; + get(); + continue; + } + + // a break marker closes the innermost indefinite-length string; + // outside of one it is not a string and falls through to the error + if (open != 0 && current == 0xFF) + { + if (--open == 0) + { + return true; + } + get(); + continue; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result))) + { + return false; + } + + if (open == 0) + { + return true; + } + + get(); + } + } + + /*! + @brief reads a definite-length CBOR byte array + + Reads everything @ref get_cbor_binary accepts except the indefinite-length + form, which that function handles itself. The bytes are appended to @a + result, so consecutive chunks of an indefinite-length byte array can be + read into the same byte array. + + @param[out] result byte array the bytes are appended to @return whether byte array creation completed - */ - bool get_cbor_binary(binary_t& result) - { - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "binary"))) - { - return false; - } + @pre @a current is not EOF + */ + bool get_cbor_binary_chunk(binary_t& result) + { switch (current) { // Binary data (0x00..0x17 bytes follow) @@ -11855,20 +11898,6 @@ class binary_reader get_binary(input_format_t::cbor, len, result); } - case 0x5F: // Binary data (indefinite length) - { - while (get() != 0xFF) - { - binary_t chunk; - if (!get_cbor_binary(chunk)) - { - return false; - } - result.insert(result.end(), chunk.begin(), chunk.end()); - } - return true; - } - default: { auto last_token = get_token_string(); @@ -11878,6 +11907,63 @@ class binary_reader } } + /*! + @brief reads a CBOR byte array + + This function first reads starting bytes to determine the expected + byte array length and then copies this number of bytes into the byte array. + Additionally, CBOR's byte arrays with indefinite lengths are supported. + + @param[out] result created byte array + + @return whether byte array creation completed + */ + bool get_cbor_binary(binary_t& result) + { + // the open indefinite-length byte arrays are counted rather than + // recursed through, for the reason given in @ref get_cbor_string + std::size_t open = 0; + + while (true) + { + if (JSON_HEDLEY_UNLIKELY(!unexpect_eof(input_format_t::cbor, "binary"))) + { + return false; + } + + if (current == 0x5F) // Binary data (indefinite length) + { + ++open; + get(); + continue; + } + + // a break marker closes the innermost indefinite-length byte + // array; outside of one it falls through to the error below + if (open != 0 && current == 0xFF) + { + if (--open == 0) + { + return true; + } + get(); + continue; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result))) + { + return false; + } + + if (open == 0) + { + return true; + } + + get(); + } + } + /*! @brief narrow a definite CBOR array/map length to std::size_t diff --git a/tests/src/unit-cbor.cpp b/tests/src/unit-cbor.cpp index 2a6bd41d7..96b30e142 100644 --- a/tests/src/unit-cbor.cpp +++ b/tests/src/unit-cbor.cpp @@ -2035,6 +2035,58 @@ TEST_CASE("CBOR definite length equal to the indefinite-length sentinel") } } +TEST_CASE("CBOR indefinite-length strings do not recurse per chunk") +{ + // Reading an indefinite-length string or byte array used to call itself + // once per chunk, so a payload of repeated 0x7F (or 0x5F) bytes exhausted + // the call stack before any of the input was rejected. The open levels are + // counted now, and the levels below prove the reader still reads the same + // values and reports the same errors at the same byte offsets. + json _; + + SECTION("many open levels are reported, not crashed on") + { + const std::vector input(200000, 0x7F); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.110] parse error at byte 200001: syntax error while parsing CBOR string: unexpected end of input", json::parse_error&); + CHECK(json::from_cbor(input, true, false).is_discarded()); + } + + SECTION("many open levels are reported, not crashed on (binary)") + { + const std::vector input(200000, 0x5F); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.110] parse error at byte 200001: syntax error while parsing CBOR binary: unexpected end of input", json::parse_error&); + CHECK(json::from_cbor(input, true, false).is_discarded()); + } + + SECTION("chunks are still concatenated") + { + CHECK(json::from_cbor(std::vector({0x7F, 0xFF})) == json("")); + CHECK(json::from_cbor(std::vector({0x7F, 0x61, 0x61, 0xFF})) == json("a")); + // nested indefinite-length strings are concatenated across levels + CHECK(json::from_cbor(std::vector({0x7F, 0x7F, 0x61, 0x61, 0xFF, 0x61, 0x62, 0xFF})) == json("ab")); + CHECK(json::from_cbor(std::vector({0x7F, 0x7F, 0x7F, 0x61, 0x7A, 0xFF, 0xFF, 0xFF})) == json("z")); + CHECK(json::from_cbor(std::vector({0xA1, 0x7F, 0x61, 0x61, 0xFF, 0x01})) == json({{"a", 1}})); + } + + SECTION("chunks are still concatenated (binary)") + { + CHECK(json::from_cbor(std::vector({0x5F, 0x41, 0x61, 0xFF})) == json::binary({0x61})); + CHECK(json::from_cbor(std::vector({0x5F, 0x5F, 0x41, 0x61, 0xFF, 0x41, 0x62, 0xFF})) == json::binary({0x61, 0x62})); + } + + SECTION("a chunk that is not a string is still rejected") + { + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x7F, 0x7F, 0x00})), "[json.exception.parse_error.113] parse error at byte 3: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x00", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F, 0x5F, 0x00})), "[json.exception.parse_error.113] parse error at byte 3: syntax error while parsing CBOR binary: expected length specification (0x40-0x5B) or indefinite binary array type (0x5F); last byte: 0x00", json::parse_error&); + } + + SECTION("a break marker outside an indefinite-length string is not a string") + { + // 0xFF only closes a string that was opened; on its own it is not one + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0xFF, 0x01})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0xFF", json::parse_error&); + } +} + TEST_CASE("CBOR roundtrips" * doctest::skip()) { SECTION("input from flynn")