From 54beb8ffae0dce512475ae940741a0bc00bcee4c Mon Sep 17 00:00:00 2001 From: "Joseph.Demarest" Date: Mon, 27 Jul 2026 12:20:47 -0400 Subject: [PATCH] fix(cbor): reject nested indefinite string chunks Signed-off-by: Joseph.Demarest Signed-off-by: Niels Lohmann --- .../docs/features/binary_formats/cbor.md | 4 + .../nlohmann/detail/input/binary_reader.hpp | 90 ++++++++++--------- single_include/nlohmann/json.hpp | 90 ++++++++++--------- tests/src/unit-cbor.cpp | 35 +++++--- tests/src/unit-regression3.cpp | 8 ++ 5 files changed, 133 insertions(+), 94 deletions(-) diff --git a/docs/mkdocs/docs/features/binary_formats/cbor.md b/docs/mkdocs/docs/features/binary_formats/cbor.md index 1b226cd15..a0a0ed8d0 100644 --- a/docs/mkdocs/docs/features/binary_formats/cbor.md +++ b/docs/mkdocs/docs/features/binary_formats/cbor.md @@ -131,6 +131,7 @@ The library maps CBOR types to JSON value types as follows: | Byte string | binary | 0x59 | | Byte string | binary | 0x5A | | Byte string | binary | 0x5B | +| Byte string | binary | 0x5F | | UTF-8 string | string | 0x60..0x77 | | UTF-8 string | string | 0x78 | | UTF-8 string | string | 0x79 | @@ -156,6 +157,9 @@ The library maps CBOR types to JSON value types as follows: | Single-Precision Float | number_float | 0xFA | | Double-Precision Float | number_float | 0xFB | +Indefinite-length UTF-8 strings (0x7F) and byte strings (0x5F) are supported. Each chunk must be a definite-length +string of the same major type, as required by [RFC 8949, Section 3.2.3](https://www.rfc-editor.org/rfc/rfc8949.html#section-3.2.3). + !!! warning "Incomplete mapping" The mapping is **incomplete** in the sense that not all CBOR types can be converted to a JSON value. The following CBOR types are not supported and will yield parse errors: diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index e6c698fa0..00c921030 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -1072,6 +1072,20 @@ class binary_reader } } + /*! + @brief reports a nested indefinite-length CBOR string or byte array + @param[in] type_name name of the rejected string type + @param[in] context parsing context for the error message + @return whether the SAX consumer accepts the parse error + */ + bool cbor_indefinite_string_error(const char* type_name, const char* context) + { + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(concat("indefinite-length ", type_name, + " is not allowed inside indefinite-length ", type_name, "; last byte: 0x", last_token), context), nullptr)); + } + /*! @brief reads a definite-length CBOR string @@ -1081,12 +1095,13 @@ class binary_reader into the same string. @param[out] result string the bytes are appended to + @param[in] is_chunk whether the bytes belong to an indefinite-length string @return whether string creation completed @pre @a current is not EOF */ - bool get_cbor_string_chunk(string_t& result) + bool get_cbor_string_chunk(string_t& result, const bool is_chunk) { switch (current) { @@ -1147,7 +1162,7 @@ class binary_reader { auto last_token = get_token_string(); return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, - exception_message(concat("expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x", last_token), "string"), nullptr)); + exception_message(concat("expected length specification (0x60-0x7B)", is_chunk ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr)); } } } @@ -1165,13 +1180,9 @@ class binary_reader */ bool get_cbor_string(string_t& result, const char* context = "string") { - // 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; + // Read chunks iteratively, but reject a second indefinite-length + // level as required by RFC 8949, Section 3.2.3. + bool indefinite = false; while (true) { @@ -1182,29 +1193,28 @@ class binary_reader 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) + if (JSON_HEDLEY_UNLIKELY(indefinite)) { - return check_string_utf8(result, context); + return cbor_indefinite_string_error("string", "string"); } + indefinite = true; get(); continue; } - if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result))) + // A break marker closes the indefinite-length string; outside + // of one it falls through to the error below. + if (indefinite && current == 0xFF) + { + return check_string_utf8(result, context); + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result, indefinite))) { return false; } - if (open == 0) + if (!indefinite) { return check_string_utf8(result, context); } @@ -1296,12 +1306,13 @@ class binary_reader read into the same byte array. @param[out] result byte array the bytes are appended to + @param[in] is_chunk whether the bytes belong to an indefinite-length string @return whether byte array creation completed @pre @a current is not EOF */ - bool get_cbor_binary_chunk(binary_t& result) + bool get_cbor_binary_chunk(binary_t& result, const bool is_chunk) { switch (current) { @@ -1366,7 +1377,7 @@ class binary_reader { auto last_token = get_token_string(); return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, - exception_message(concat("expected length specification (0x40-0x5B) or indefinite binary array type (0x5F); last byte: 0x", last_token), "binary"), nullptr)); + exception_message(concat("expected length specification (0x40-0x5B)", is_chunk ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr)); } } } @@ -1384,9 +1395,9 @@ class binary_reader */ 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; + // Read chunks iteratively, but reject a second indefinite-length + // level as required by RFC 8949, Section 3.2.3. + bool indefinite = false; while (true) { @@ -1397,29 +1408,28 @@ class binary_reader 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) + if (JSON_HEDLEY_UNLIKELY(indefinite)) { - return true; + return cbor_indefinite_string_error("binary array", "binary"); } + indefinite = true; get(); continue; } - if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result))) + // A break marker closes the indefinite-length string; outside + // of one it falls through to the error below. + if (indefinite && current == 0xFF) + { + return true; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result, indefinite))) { return false; } - if (open == 0) + if (!indefinite) { return true; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 86321a36e..40633ba94 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -14789,6 +14789,20 @@ class binary_reader } } + /*! + @brief reports a nested indefinite-length CBOR string or byte array + @param[in] type_name name of the rejected string type + @param[in] context parsing context for the error message + @return whether the SAX consumer accepts the parse error + */ + bool cbor_indefinite_string_error(const char* type_name, const char* context) + { + auto last_token = get_token_string(); + return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, + exception_message(concat("indefinite-length ", type_name, + " is not allowed inside indefinite-length ", type_name, "; last byte: 0x", last_token), context), nullptr)); + } + /*! @brief reads a definite-length CBOR string @@ -14798,12 +14812,13 @@ class binary_reader into the same string. @param[out] result string the bytes are appended to + @param[in] is_chunk whether the bytes belong to an indefinite-length string @return whether string creation completed @pre @a current is not EOF */ - bool get_cbor_string_chunk(string_t& result) + bool get_cbor_string_chunk(string_t& result, const bool is_chunk) { switch (current) { @@ -14864,7 +14879,7 @@ class binary_reader { auto last_token = get_token_string(); return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, - exception_message(concat("expected length specification (0x60-0x7B) or indefinite string type (0x7F); last byte: 0x", last_token), "string"), nullptr)); + exception_message(concat("expected length specification (0x60-0x7B)", is_chunk ? "" : " or indefinite string type (0x7F)", "; last byte: 0x", last_token), "string"), nullptr)); } } } @@ -14882,13 +14897,9 @@ class binary_reader */ bool get_cbor_string(string_t& result, const char* context = "string") { - // 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; + // Read chunks iteratively, but reject a second indefinite-length + // level as required by RFC 8949, Section 3.2.3. + bool indefinite = false; while (true) { @@ -14899,29 +14910,28 @@ class binary_reader 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) + if (JSON_HEDLEY_UNLIKELY(indefinite)) { - return check_string_utf8(result, context); + return cbor_indefinite_string_error("string", "string"); } + indefinite = true; get(); continue; } - if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result))) + // A break marker closes the indefinite-length string; outside + // of one it falls through to the error below. + if (indefinite && current == 0xFF) + { + return check_string_utf8(result, context); + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_string_chunk(result, indefinite))) { return false; } - if (open == 0) + if (!indefinite) { return check_string_utf8(result, context); } @@ -15013,12 +15023,13 @@ class binary_reader read into the same byte array. @param[out] result byte array the bytes are appended to + @param[in] is_chunk whether the bytes belong to an indefinite-length string @return whether byte array creation completed @pre @a current is not EOF */ - bool get_cbor_binary_chunk(binary_t& result) + bool get_cbor_binary_chunk(binary_t& result, const bool is_chunk) { switch (current) { @@ -15083,7 +15094,7 @@ class binary_reader { auto last_token = get_token_string(); return sax->parse_error(chars_read, last_token, parse_error::create(113, chars_read, - exception_message(concat("expected length specification (0x40-0x5B) or indefinite binary array type (0x5F); last byte: 0x", last_token), "binary"), nullptr)); + exception_message(concat("expected length specification (0x40-0x5B)", is_chunk ? "" : " or indefinite binary array type (0x5F)", "; last byte: 0x", last_token), "binary"), nullptr)); } } } @@ -15101,9 +15112,9 @@ class binary_reader */ 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; + // Read chunks iteratively, but reject a second indefinite-length + // level as required by RFC 8949, Section 3.2.3. + bool indefinite = false; while (true) { @@ -15114,29 +15125,28 @@ class binary_reader 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) + if (JSON_HEDLEY_UNLIKELY(indefinite)) { - return true; + return cbor_indefinite_string_error("binary array", "binary"); } + indefinite = true; get(); continue; } - if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result))) + // A break marker closes the indefinite-length string; outside + // of one it falls through to the error below. + if (indefinite && current == 0xFF) + { + return true; + } + + if (JSON_HEDLEY_UNLIKELY(!get_cbor_binary_chunk(result, indefinite))) { return false; } - if (open == 0) + if (!indefinite) { return true; } diff --git a/tests/src/unit-cbor.cpp b/tests/src/unit-cbor.cpp index 38763b4db..c888883f3 100644 --- a/tests/src/unit-cbor.cpp +++ b/tests/src/unit-cbor.cpp @@ -1699,7 +1699,7 @@ TEST_CASE("CBOR") CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0x61, 0X61})), "[json.exception.parse_error.110] parse error at byte 4: syntax error while parsing CBOR value: unexpected end of input", json::parse_error&); CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xBF, 0x61, 0X61})), "[json.exception.parse_error.110] parse error at byte 4: syntax error while parsing CBOR value: unexpected end of input", json::parse_error&); CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F})), "[json.exception.parse_error.110] parse error at byte 2: syntax error while parsing CBOR binary: unexpected end of input", json::parse_error&); - CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F, 0x00})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: expected length specification (0x40-0x5B) or indefinite binary array type (0x5F); last byte: 0x00", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F, 0x00})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: expected length specification (0x40-0x5B); last byte: 0x00", json::parse_error&); CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x41})), "[json.exception.parse_error.110] parse error at byte 2: syntax error while parsing CBOR binary: unexpected end of input", json::parse_error&); CHECK(json::from_cbor(std::vector({0x18}), true, false).is_discarded()); @@ -2305,22 +2305,21 @@ 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. + // the call stack before any of the input was rejected. Nested indefinite + // chunks are now rejected at the second byte, without recursing. 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_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: indefinite-length string is not allowed inside indefinite-length string; last byte: 0x7F", 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_THROWS_WITH_AS(_ = json::from_cbor(input), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&); CHECK(json::from_cbor(input, true, false).is_discarded()); } @@ -2328,22 +2327,22 @@ TEST_CASE("CBOR indefinite-length strings do not recurse per chunk") { 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")); + // Empty and nonempty definite-length chunks concatenate in order. + CHECK(json::from_cbor(std::vector({0x7F, 0x61, 'a', 0x60, 0x61, 'b', 0x61, 'c', 0xFF})) == json("abc")); 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})); + CHECK(json::from_cbor(std::vector({0x5F, 0xFF})) == json::binary({})); + CHECK(json::from_cbor(std::vector({0x5F, 0x41, 0x61, 0x40, 0x41, 0x62, 0x41, 0x63, 0xFF})) == json::binary({0x61, 0x62, 0x63})); } 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&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x7F, 0x00})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: expected length specification (0x60-0x7B); last byte: 0x00", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F, 0x00})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: expected length specification (0x40-0x5B); last byte: 0x00", json::parse_error&); } SECTION("a break marker outside an indefinite-length string is not a string") @@ -2896,9 +2895,17 @@ TEST_CASE("examples from RFC 8949 Appendix A") { const auto packed = utils::read_binary_file(TEST_DATA_DIRECTORY "/binary_data/cbor_binary.cbor"); json j; - CHECK_NOTHROW(j = json::from_cbor(packed)); + // The fixture's tail contains nested indefinite-length byte strings. + CHECK_THROWS_WITH_AS(j = json::from_cbor(packed), "[json.exception.parse_error.113] parse error at byte 513: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&); - const auto expected = utils::read_binary_file(TEST_DATA_DIRECTORY "/binary_data/cbor_binary.out"); + // Keep the byte-for-byte decoding check for its valid prefix: the first + // 512 encoded bytes contain 468 payload bytes in definite-length chunks. + auto valid_prefix = packed; + valid_prefix.resize(512); + valid_prefix.push_back(0xFF); + auto expected = utils::read_binary_file(TEST_DATA_DIRECTORY "/binary_data/cbor_binary.out"); + expected.resize(468); + CHECK_NOTHROW(j = json::from_cbor(valid_prefix)); CHECK(j == json::binary(expected)); // 0xd8 diff --git a/tests/src/unit-regression3.cpp b/tests/src/unit-regression3.cpp index 95b2b6d4f..200474714 100644 --- a/tests/src/unit-regression3.cpp +++ b/tests/src/unit-regression3.cpp @@ -920,4 +920,12 @@ TEST_CASE("regression test #5476 - array type without reserve()") } } +TEST_CASE("issue #5317 - nested indefinite-length CBOR string chunks are rejected") +{ + json _; + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x7F, 0x7F, 0x61, 0x61, 0xFF, 0xFF})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR string: indefinite-length string is not allowed inside indefinite-length string; last byte: 0x7F", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0x5F, 0x5F, 0x41, 0x61, 0xFF, 0xFF})), "[json.exception.parse_error.113] parse error at byte 2: syntax error while parsing CBOR binary: indefinite-length binary array is not allowed inside indefinite-length binary array; last byte: 0x5F", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_cbor(std::vector({0xA1, 0x7F, 0x7F, 0xFF, 0xFF, 0x01})), "[json.exception.parse_error.113] parse error at byte 3: syntax error while parsing CBOR string: indefinite-length string is not allowed inside indefinite-length string; last byte: 0x7F", json::parse_error&); +} + DOCTEST_CLANG_SUPPRESS_WARNING_POP