From dc768a7d5da7e789e85514510915a029bbbb5b8e Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 26 Oct 2026 18:25:46 +0100 Subject: [PATCH] Fix BSON binary subtype 0x02 and check the declared document size earlier (#5930) * Write and read the inner length of BSON binary subtype 0x02 The BSON specification defines the payload of the "binary (old)" subtype 0x02 as an int32 followed by that many bytes. to_bson wrote the plain payload instead, so decoders such as pymongo and libbson-based readers rejected the bytes, and from_bson returned the inner length as part of the payload (documented as lenient behavior in gh-5333). The writer now emits the outer length (payload + 4), the subtype, the inner int32 holding the payload size, and the payload. The reader requires an outer length of at least 4 and an inner length equal to the outer length minus 4 (the cases the bson-corpus lists as decode errors), reports parse error 112 otherwise, and returns only the payload. Both directions change together so that the round trip keeps working. The single-reservation fast path for scalar entries declines subtype 0x02 because it has no room for the inner length. This is a deliberate exception to the opt-in rule for behavior changes: the previous output was invalid BSON. Signed-off-by: Niels Lohmann * Check the declared BSON document size before reading its content The BSON reader compared a document's declared size with the number of bytes read only when it reached the terminator. The declared size never bounded the reads, so a SAX handler saw the keys and values of a document whose size was too small before parse error 112 was reported. The container frame now stores the position where the document ends according to its declared size. open_bson_document rejects a size below 5 (size prefix plus terminator) and an embedded document that does not end before the terminator of its enclosing document; the element loop rejects an element whose type byte leaves no room for the name terminator and the document terminator. The terminator check stays and still catches a declared size that is too large. No input that was accepted before is rejected now; documents with a wrong size are reported earlier, with fewer SAX events, a different byte position and, in three cases, a different message. The error id (parse_error.112) is unchanged and strict=false still does not skip the check. The tests that asserted the old messages and positions are updated, with the outer sizes of two inputs corrected so that they still exercise the embedded document. New tests cover the SAX events, a size below 5, and an embedded document that fits exactly. Signed-off-by: Niels Lohmann * Document BSON subtype 0x02, document size checks, and lenient reads Describe the inner length of binary subtype 0x02 that to_bson now writes and from_bson checks and removes. Add the document size checks (parse_error.112, not affected by strict=false) to bson.md and from_bson.md, and extend the list of lenient reads with the UUID and MD5 payload lengths (not checked), duplicate keys (last value wins), embedded NUL bytes in strings, and the ignored trailing bytes with strict=false. State that the members of an object are written in iteration order. Correct the statement that element names are never validated on read: they are checked and sanitized like string values by the error handler. Add the BSON message to the examples of out_of_range.415 and type_error.321, the subtype limit of 255 to the binary values page, CBOR to the formats that use byte_container_with_subtype, and fix a typo in the incomplete mapping box. Signed-off-by: Niels Lohmann * Regenerate single_include and apply astyle Result of make amalgamate (including make pretty) for the preceding changes. Signed-off-by: Niels Lohmann * Address review of the BSON documentation Describe exactly when a too small document size is detected and note that subtype 2 data written by earlier versions is not readable. Signed-off-by: Niels Lohmann * docs: use the Oxford comma, drop discarded-value remarks Add the serial comma in a list added by this PR and remove the BSON example of type_error.321 (with the type_error.317 remark) so the docs do not mention discarded values for BSON; restore the original singular heading of that section. No tables were changed by this PR. Signed-off-by: Niels Lohmann * Fix -Weffc++ in the BSON recorder test and a clang-tidy use-auto hit Signed-off-by: Niels Lohmann * Fix clang-tidy readability-redundant-member-init in BSON test Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- docs/mkdocs/docs/api/basic_json/from_bson.md | 3 +- docs/mkdocs/docs/api/basic_json/to_bson.md | 2 +- .../api/byte_container_with_subtype/index.md | 2 +- .../docs/features/binary_formats/bson.md | 46 ++- docs/mkdocs/docs/features/binary_values.md | 5 +- docs/mkdocs/docs/home/exceptions.md | 6 +- .../nlohmann/detail/input/binary_reader.hpp | 71 +++++ .../nlohmann/detail/output/binary_writer.hpp | 27 +- single_include/nlohmann/json.hpp | 98 ++++++- tests/src/unit-bson.cpp | 266 ++++++++++++++++-- 10 files changed, 491 insertions(+), 35 deletions(-) diff --git a/docs/mkdocs/docs/api/basic_json/from_bson.md b/docs/mkdocs/docs/api/basic_json/from_bson.md index 6e0beafce..c04933eef 100644 --- a/docs/mkdocs/docs/api/basic_json/from_bson.md +++ b/docs/mkdocs/docs/api/basic_json/from_bson.md @@ -80,7 +80,8 @@ Strong guarantee: if an exception is thrown, there are no changes in the JSON va - Throws [`parse_error.110`](../../home/exceptions.md#jsonexceptionparse_error110) if the given input ends prematurely or the end of the input was not reached when `strict` was set to true - Throws [`parse_error.112`](../../home/exceptions.md#jsonexceptionparse_error112) if a parse error occurs (e.g., an - invalid string or byte array length) + invalid string or byte array length, an invalid inner length of a binary value with subtype `0x02`, or a document size + that does not match the content; `strict` does not affect the document size checks) - Throws [`parse_error.114`](../../home/exceptions.md#jsonexceptionparse_error114) if an unsupported BSON record type is encountered - Throws [`parse_error.113`](../../home/exceptions.md#jsonexceptionparse_error113) if a string value or object key is diff --git a/docs/mkdocs/docs/api/basic_json/to_bson.md b/docs/mkdocs/docs/api/basic_json/to_bson.md index 315477e19..b3b346266 100644 --- a/docs/mkdocs/docs/api/basic_json/to_bson.md +++ b/docs/mkdocs/docs/api/basic_json/to_bson.md @@ -13,7 +13,7 @@ static void to_bson(const basic_json& j, detail::output_adapter o, ``` BSON (Binary JSON) is a binary format in which zero or more ordered key/value pairs are stored as a single entity (a -so-called document). +so-called document). The members of an object are written in the order in which iteration visits them. 1. Returns a byte vector containing the BSON serialization. 2. Writes the BSON serialization to an output adapter. diff --git a/docs/mkdocs/docs/api/byte_container_with_subtype/index.md b/docs/mkdocs/docs/api/byte_container_with_subtype/index.md index b184d8496..f3a2cc9a3 100644 --- a/docs/mkdocs/docs/api/byte_container_with_subtype/index.md +++ b/docs/mkdocs/docs/api/byte_container_with_subtype/index.md @@ -6,7 +6,7 @@ class byte_container_with_subtype : public BinaryType; ``` This type extends the template parameter `BinaryType` provided to [`basic_json`](../basic_json/index.md) with a subtype -used by BSON and MessagePack. This type exists so that the user does not have to specify a type themselves with a +used by BSON, CBOR, and MessagePack. This type exists so that the user does not have to specify a type themselves with a specific naming scheme in order to override the binary type. ## Template parameters diff --git a/docs/mkdocs/docs/features/binary_formats/bson.md b/docs/mkdocs/docs/features/binary_formats/bson.md index 9c82594dd..04aef6950 100644 --- a/docs/mkdocs/docs/features/binary_formats/bson.md +++ b/docs/mkdocs/docs/features/binary_formats/bson.md @@ -66,7 +66,12 @@ The library uses the following mapping from JSON values types to BSON types: !!! warning "Incomplete mapping" The mapping is **incomplete**, since only JSON-objects (and things contained therein) can be serialized to BSON. - Also, keys may not contain U+0000, since they are serialized a zero-terminated c-strings. + Also, keys may not contain U+0000, since they are serialized as zero-terminated C strings. + +!!! info "Order of object members" + + The members of an object are written in the order in which iteration visits them: sorted by key for + [`json`](../../api/json.md), in insertion order for [`ordered_json`](../../api/ordered_json.md). !!! warning "BSON type 0x11 interoperability" @@ -81,6 +86,19 @@ The library uses the following mapping from JSON values types to BSON types: As a result, serializing and deserializing a JSON object containing such a value produces a different JSON object, even though the binary data is unchanged. +!!! info "Binary subtype 0x02 (old binary)" + + The BSON specification defines the payload of the "binary (old)" subtype `0x02` as an int32 followed by that many + bytes, so the value is preceded by two lengths: the length of the binary element and, as the first four bytes of + the payload, the number of bytes that follow. A binary value with subtype `2` is therefore written with the outer + length 4 bytes larger than its size and the size as inner length: `binary({1, 2, 3}, 2)` becomes + `07 00 00 00 02 03 00 00 00 01 02 03`. When reading, the inner length is checked and removed, so the value + contains only the bytes that follow it. `from_bson()` throws + [`parse_error.112`](../../home/exceptions.md#jsonexceptionparse_error112) if the outer length is less than 4 or the + inner length is not the outer length minus 4. Versions before 3.13.0 did not write the inner length and returned + it as part of the payload; BSON data written by those versions with subtype 2 is not readable any more and must be + written again. + ??? example "Example: serialize a JSON value to BSON" ```cpp @@ -135,12 +153,29 @@ The library maps BSON record types to JSON value types as follows: The BSON reader is lenient in a few areas where the BSON specification is more restrictive: - array element keys are not checked against the required decimal sequence (`0`, `1`, `2`, ...), - - any non-zero byte is accepted as `true` for the boolean type, and - - the payload for binary subtype `0x02` is returned as-is, including its inner length prefix. + - any non-zero byte is accepted as `true` for the boolean type, + - the payloads of the binary subtypes `0x03` (UUID, old), `0x04` (UUID), and `0x05` (MD5) are not checked to have + 16 bytes, + - duplicate keys within a document are accepted; the last value wins (`{"a": 1, "a": 2}` is read as `{"a": 2}`), + while a SAX handler receives every occurrence, + - a `string` value may contain embedded `0x00` bytes, and + - with `strict` set to `false`, bytes after the end of the document are ignored. If BSON input must be validated for strict specification compliance, validate it separately before passing it to `from_bson()`. +!!! note "Document size" + + Every document starts with its size. `from_bson()` rejects a document with + [`parse_error.112`](../../home/exceptions.md#jsonexceptionparse_error112) if the size is less than 5 (the size prefix and the terminator), if an embedded document does not end before the + end of the document that contains it, if the elements do not fit into the declared size, or if the size does not + match the number of bytes read when the terminating `0x00` byte is reached. A size that is too small is detected + before an element is read whose type byte and name no longer fit; an element that starts inside the declared size + but extends past it is read, and reported to a SAX handler, before the mismatch is found at the terminator. A size + that is too large is likewise found only at the terminator. Passing `strict` as `false` skips only the check for bytes after + the end of the document ([`parse_error.110`](../../home/exceptions.md#jsonexceptionparse_error110)), not the + document size checks. + !!! warning "Ill-formed UTF-8 in string values" The BSON specification requires `string` values (type `0x02`) to be valid UTF-8, but this is not required of a @@ -154,8 +189,9 @@ The library maps BSON record types to JSON value types as follows: ill-formed bytes. `to_bson()`'s own `error_handler` parameter defaults to `keep`, so such a string value or element (key) name is written unchanged; with `strict` (the default if [`JSON_STRICT_BINARY_UTF8`](../../api/macros/json_strict_binary_utf8.md) is enabled), it throws the same exception - instead. Element (key) names are never validated on read, since they are read byte-by-byte as a C string. `binary` - values (type `0x05`) are unaffected, since they are not required to hold text. + instead. Element (key) names are treated like string values: with `keep` they are read unchanged, with `strict`, + `replace`, or `ignore` they are checked and handled in the same way. `binary` values (type `0x05`) are unaffected, + since they are not required to hold text. ??? example "Example: deserialize a JSON value from BSON" diff --git a/docs/mkdocs/docs/features/binary_values.md b/docs/mkdocs/docs/features/binary_values.md index f5a87472b..e4dee45d4 100644 --- a/docs/mkdocs/docs/features/binary_values.md +++ b/docs/mkdocs/docs/features/binary_values.md @@ -227,7 +227,10 @@ array of integers. ### BSON [BSON](binary_formats/bson.md) supports binary values and subtypes. If a subtype is given, it is used and added as an -unsigned 8-bit integer. If no subtype is given, the generic binary subtype 0x00 is used. +unsigned 8-bit integer; a subtype above 255 throws +[`out_of_range.415`](../home/exceptions.md#jsonexceptionout_of_range415). If no subtype is given, the generic binary +subtype 0x00 is used. The payload of the "old binary" subtype 0x02 is preceded by a second length, which the library +writes and removes again when reading; it is not part of the payload. ??? example "Example: serialize a binary value to BSON" diff --git a/docs/mkdocs/docs/home/exceptions.md b/docs/mkdocs/docs/home/exceptions.md index ddf9d69cd..48825cb25 100644 --- a/docs/mkdocs/docs/home/exceptions.md +++ b/docs/mkdocs/docs/home/exceptions.md @@ -1162,12 +1162,16 @@ A JSON Patch `move` operation's `"from"` location is a proper prefix of its `"pa MessagePack's ext type and BSON's binary subtype are each stored in a single byte. This exception is thrown when serializing a [`byte_container_with_subtype`](../api/byte_container_with_subtype/index.md) whose subtype exceeds 255, the largest value the library can store in that byte. -!!! failure "Example message" +!!! failure "Example messages" ``` [json.exception.out_of_range.415] subtype 70000 is too large for the MessagePack ext type (max 255) ``` + ``` + [json.exception.out_of_range.415] subtype 70000 is too large for the BSON binary subtype (max 255) + ``` + !!! note This exception was added in version 3.13.0. Before that, subtypes above 255 were silently truncated modulo 256 instead of raising an error. diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 4c8d6ed0f..b44fd868c 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -235,6 +235,9 @@ class binary_reader /// UBJSON/BJData: the type marker of an optimized container, so that /// its elements are read without one of their own; 0 otherwise char_int_type type_marker; + /// BSON: the position right after this document, according to the + /// size it declares + std::size_t end_position = 0; /// BSON: the size this document declares, in bytes std::int32_t declared_size = 0; /// whether to close this container with end_object() or end_array() @@ -371,6 +374,23 @@ class binary_reader return false; } + // the smallest document is the size prefix and the terminator + if (JSON_HEDLEY_UNLIKELY(document_size < 5)) + { + return bson_document_size_error("document size ", document_size, " is less than 5"); + } + + // the declared size bounds where the document ends: an embedded + // document must end before the terminator of the document that contains it + const auto unsigned_size = static_cast(document_size); + const std::size_t document_end = unsigned_size > (std::numeric_limits::max)() - document_start + ? (std::numeric_limits::max)() + : document_start + unsigned_size; + if (JSON_HEDLEY_UNLIKELY(!container_stack.empty() && document_end >= container_stack.back().end_position)) + { + return bson_document_size_error("document size ", document_size, " exceeds the size of the enclosing document"); + } + if (JSON_HEDLEY_UNLIKELY(!enter_container(is_object, detail::unknown_size()))) { return false; @@ -378,6 +398,7 @@ class binary_reader container_frame& frame = container_stack.back(); frame.start_position = document_start; + frame.end_position = document_end; frame.declared_size = document_size; return true; } @@ -436,6 +457,13 @@ class binary_reader return false; } + // an element needs at least its name's terminator, and the + // document its own terminator, within the declared size + if (JSON_HEDLEY_UNLIKELY(chars_read + 2 > container_stack.back().end_position)) + { + return bson_document_size_error("document size ", container_stack.back().declared_size, " is too small for its content"); + } + const std::size_t element_type_parse_position = chars_read; key.clear(); if (JSON_HEDLEY_UNLIKELY(!get_bson_cstr(key))) @@ -584,9 +612,46 @@ class binary_reader } result.set_subtype(subtype); + // the "old binary" subtype 0x02 starts with an int32 that repeats the + // size of the payload following it; the payload does not include it + if (subtype == 0x02) + { + return get_bson_old_binary(len, result); + } + return get_binary(len, result); } + /*! + @brief Parses the payload of a BSON binary value with subtype 0x02. + @param[in] len The length of the binary value, including the inner int32. + @param[in,out] result A reference to the binary variable where the payload + (without the inner int32) is to be stored. + @pre len >= 0 + @return `true` if the inner length was present and matches @a len + */ + template + bool get_bson_old_binary(const NumberType len, binary_t& result) + { + if (JSON_HEDLEY_UNLIKELY(len < 4)) + { + return bson_length_error("length of binary subtype 0x02 must be at least 4, is ", len, "binary"); + } + + std::int32_t inner_length{}; + if (JSON_HEDLEY_UNLIKELY((!get_number(inner_length)))) + { + return false; + } + + if (JSON_HEDLEY_UNLIKELY(static_cast(inner_length) != static_cast(len) - 4)) + { + return bson_length_error("inner length of binary subtype 0x02 does not match the payload size; inner length is ", inner_length, "binary"); + } + + return get_binary(inner_length, result); + } + /*! @brief Read a BSON document element of the given @a element_type. @param[in] element_type The BSON element type, c.f. http://bsonspec.org/spec.html @@ -4399,6 +4464,12 @@ class binary_reader return last_byte_error(exception_id::unexpected_byte, concat("document size ", std::to_string(document_size), " does not match the number of bytes read (", std::to_string(bytes_read), ")"), "document"); } + JSON_HEDLEY_NEVER_INLINE + bool bson_document_size_error(const char* prefix, const std::int32_t document_size, const char* suffix) const + { + return last_byte_error(exception_id::unexpected_byte, concat(prefix, std::to_string(document_size), suffix), "document"); + } + JSON_HEDLEY_NEVER_INLINE bool bson_unsupported_type_error(const std::size_t position, const char_int_type element_type) const { diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 7ff828074..d8b3e7162 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -1684,7 +1684,15 @@ class binary_writer throw_subtype_out_of_range(value.subtype(), "the BSON binary subtype", j); } - return sizeof(std::int32_t) + value.size() + 1ul; + // the "old binary" subtype 0x02 repeats the payload size in an inner + // int32 in front of the payload + return sizeof(std::int32_t) + (is_bson_old_binary(value) ? sizeof(std::int32_t) : 0ul) + value.size() + 1ul; + } + + /// @return whether @a value has the BSON subtype 0x02 ("binary (old)") + static bool is_bson_old_binary(const binary_t& value) noexcept + { + return value.has_subtype() && value.subtype() == 0x02; } /*! @@ -1697,10 +1705,18 @@ class binary_writer { write_bson_entry_header(name, 0x05); - write_number(to_bson_length(value.size()), true); + const bool old_binary = is_bson_old_binary(value); + + // the length field counts the inner int32 of the old binary subtype + write_number(to_bson_length(value.size() + (old_binary ? sizeof(std::int32_t) : 0ul)), true); write_number(value.has_subtype() ? static_cast(value.subtype()) : static_cast(0x00)); + if (old_binary) + { + write_number(to_bson_length(value.size()), true); + } + write_bytes(value.data(), value.size()); } @@ -1860,7 +1876,8 @@ class binary_writer (or the header of a string or binary value) in one reservation @return whether the entry was written; false for a discarded value, a string or binary value whose length does not fit the length - field, in which case nothing was written + field, or a binary value with subtype 0x02, in which case nothing + was written @pre @a name is at most bson_scalar_max_name_size characters long and is not changed by the error handler */ @@ -1921,7 +1938,9 @@ class binary_writer if (j.is_binary()) { const binary_t& value = *j.m_data.m_value.binary; - if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()))) + // the old binary subtype 0x02 has an inner length that the single + // reservation has no room for; write_bson_binary writes it + if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()) || is_bson_old_binary(value))) { return false; } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index c4b7f159a..b67223f0c 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -17702,6 +17702,9 @@ class binary_reader /// UBJSON/BJData: the type marker of an optimized container, so that /// its elements are read without one of their own; 0 otherwise char_int_type type_marker; + /// BSON: the position right after this document, according to the + /// size it declares + std::size_t end_position = 0; /// BSON: the size this document declares, in bytes std::int32_t declared_size = 0; /// whether to close this container with end_object() or end_array() @@ -17838,6 +17841,23 @@ class binary_reader return false; } + // the smallest document is the size prefix and the terminator + if (JSON_HEDLEY_UNLIKELY(document_size < 5)) + { + return bson_document_size_error("document size ", document_size, " is less than 5"); + } + + // the declared size bounds where the document ends: an embedded + // document must end before the terminator of the document that contains it + const auto unsigned_size = static_cast(document_size); + const std::size_t document_end = unsigned_size > (std::numeric_limits::max)() - document_start + ? (std::numeric_limits::max)() + : document_start + unsigned_size; + if (JSON_HEDLEY_UNLIKELY(!container_stack.empty() && document_end >= container_stack.back().end_position)) + { + return bson_document_size_error("document size ", document_size, " exceeds the size of the enclosing document"); + } + if (JSON_HEDLEY_UNLIKELY(!enter_container(is_object, detail::unknown_size()))) { return false; @@ -17845,6 +17865,7 @@ class binary_reader container_frame& frame = container_stack.back(); frame.start_position = document_start; + frame.end_position = document_end; frame.declared_size = document_size; return true; } @@ -17903,6 +17924,13 @@ class binary_reader return false; } + // an element needs at least its name's terminator, and the + // document its own terminator, within the declared size + if (JSON_HEDLEY_UNLIKELY(chars_read + 2 > container_stack.back().end_position)) + { + return bson_document_size_error("document size ", container_stack.back().declared_size, " is too small for its content"); + } + const std::size_t element_type_parse_position = chars_read; key.clear(); if (JSON_HEDLEY_UNLIKELY(!get_bson_cstr(key))) @@ -18051,9 +18079,46 @@ class binary_reader } result.set_subtype(subtype); + // the "old binary" subtype 0x02 starts with an int32 that repeats the + // size of the payload following it; the payload does not include it + if (subtype == 0x02) + { + return get_bson_old_binary(len, result); + } + return get_binary(len, result); } + /*! + @brief Parses the payload of a BSON binary value with subtype 0x02. + @param[in] len The length of the binary value, including the inner int32. + @param[in,out] result A reference to the binary variable where the payload + (without the inner int32) is to be stored. + @pre len >= 0 + @return `true` if the inner length was present and matches @a len + */ + template + bool get_bson_old_binary(const NumberType len, binary_t& result) + { + if (JSON_HEDLEY_UNLIKELY(len < 4)) + { + return bson_length_error("length of binary subtype 0x02 must be at least 4, is ", len, "binary"); + } + + std::int32_t inner_length{}; + if (JSON_HEDLEY_UNLIKELY((!get_number(inner_length)))) + { + return false; + } + + if (JSON_HEDLEY_UNLIKELY(static_cast(inner_length) != static_cast(len) - 4)) + { + return bson_length_error("inner length of binary subtype 0x02 does not match the payload size; inner length is ", inner_length, "binary"); + } + + return get_binary(inner_length, result); + } + /*! @brief Read a BSON document element of the given @a element_type. @param[in] element_type The BSON element type, c.f. http://bsonspec.org/spec.html @@ -21866,6 +21931,12 @@ class binary_reader return last_byte_error(exception_id::unexpected_byte, concat("document size ", std::to_string(document_size), " does not match the number of bytes read (", std::to_string(bytes_read), ")"), "document"); } + JSON_HEDLEY_NEVER_INLINE + bool bson_document_size_error(const char* prefix, const std::int32_t document_size, const char* suffix) const + { + return last_byte_error(exception_id::unexpected_byte, concat(prefix, std::to_string(document_size), suffix), "document"); + } + JSON_HEDLEY_NEVER_INLINE bool bson_unsupported_type_error(const std::size_t position, const char_int_type element_type) const { @@ -28045,7 +28116,15 @@ class binary_writer throw_subtype_out_of_range(value.subtype(), "the BSON binary subtype", j); } - return sizeof(std::int32_t) + value.size() + 1ul; + // the "old binary" subtype 0x02 repeats the payload size in an inner + // int32 in front of the payload + return sizeof(std::int32_t) + (is_bson_old_binary(value) ? sizeof(std::int32_t) : 0ul) + value.size() + 1ul; + } + + /// @return whether @a value has the BSON subtype 0x02 ("binary (old)") + static bool is_bson_old_binary(const binary_t& value) noexcept + { + return value.has_subtype() && value.subtype() == 0x02; } /*! @@ -28058,10 +28137,18 @@ class binary_writer { write_bson_entry_header(name, 0x05); - write_number(to_bson_length(value.size()), true); + const bool old_binary = is_bson_old_binary(value); + + // the length field counts the inner int32 of the old binary subtype + write_number(to_bson_length(value.size() + (old_binary ? sizeof(std::int32_t) : 0ul)), true); write_number(value.has_subtype() ? static_cast(value.subtype()) : static_cast(0x00)); + if (old_binary) + { + write_number(to_bson_length(value.size()), true); + } + write_bytes(value.data(), value.size()); } @@ -28221,7 +28308,8 @@ class binary_writer (or the header of a string or binary value) in one reservation @return whether the entry was written; false for a discarded value, a string or binary value whose length does not fit the length - field, in which case nothing was written + field, or a binary value with subtype 0x02, in which case nothing + was written @pre @a name is at most bson_scalar_max_name_size characters long and is not changed by the error handler */ @@ -28282,7 +28370,9 @@ class binary_writer if (j.is_binary()) { const binary_t& value = *j.m_data.m_value.binary; - if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()))) + // the old binary subtype 0x02 has an inner length that the single + // reservation has no room for; write_bson_binary writes it + if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()) || is_bson_old_binary(value))) { return false; } diff --git a/tests/src/unit-bson.cpp b/tests/src/unit-bson.cpp index 03c901cee..b4756c9b5 100644 --- a/tests/src/unit-bson.cpp +++ b/tests/src/unit-bson.cpp @@ -802,12 +802,11 @@ TEST_CASE("BSON") CHECK(json::from_bson(result, true, false) == j); } - SECTION("binary member with subtype 0x02 (old binary) keeps its inner length prefix (lenient parsing)") + SECTION("binary member with subtype 0x02 (old binary) carries an inner length") { - // documented lenient behavior (see gh-5333): the payload for - // binary subtype 0x02 ("old binary") is returned as-is, - // including its own inner 4-byte length prefix; it is not - // stripped or reinterpreted + // the payload of binary subtype 0x02 ("old binary") starts with an + // int32 that repeats the payload size; the bytes below are what + // pymongo and the bson-corpus (binary.json) use std::vector const input = { 0x17, 0x00, 0x00, 0x00, // size (little endian) @@ -815,16 +814,87 @@ TEST_CASE("BSON") 0x06, 0x00, 0x00, 0x00, // size of binary (little endian) 0x02, // "old binary" subtype - 0x02, 0x00, 0x00, 0x00, // inner length prefix (part of the old-binary payload) + 0x02, 0x00, 0x00, 0x00, // inner length 0x68, 0x69, // payload ('h', 'i') 0x00 // end marker }; - // the inner length prefix is part of the (unmodified) payload - const std::vector expected_payload = {0x02, 0x00, 0x00, 0x00, 0x68, 0x69}; - const json expected = { { "entry", json::binary(expected_payload, 0x02) } }; + // the inner length is not part of the payload + const json expected = { { "entry", json::binary({0x68, 0x69}, 0x02) } }; CHECK(json::from_bson(input) == expected); + CHECK(json::to_bson(expected) == input); + CHECK(json::from_bson(json::to_bson(expected)) == expected); + } + + SECTION("binary member with subtype 0x02 (old binary) and empty payload") + { + std::vector const input = + { + 0x15, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x04, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x00, 0x00, 0x00, 0x00, // inner length + 0x00 // end marker + }; + + const json expected = { { "entry", json::binary({}, 0x02) } }; + CHECK(json::from_bson(input) == expected); + CHECK(json::to_bson(expected) == input); + } + + SECTION("binary member with subtype 0x02 (old binary) with an invalid inner length") + { + std::vector input = + { + 0x17, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x06, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x02, 0x00, 0x00, 0x00, // inner length + 0x68, 0x69, // payload ('h', 'i') + 0x00 // end marker + }; + + SECTION("inner length too long") + { + input[19] = 0x03; + } + + SECTION("inner length too short") + { + input[19] = 0x01; + } + + SECTION("inner length negative") + { + input[19] = 0xFF; + input[20] = 0xFF; + input[21] = 0xFF; + input[22] = 0xFF; + } + + CAPTURE(input) + json _; + CHECK_THROWS_AS(_ = json::from_bson(input), json::parse_error&); + } + + SECTION("binary member with subtype 0x02 (old binary) without an inner length") + { + // the payload is shorter than the inner length field + std::vector const input = + { + 0x14, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x03, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x01, 0x02, 0x03, // payload without an inner length + 0x00 // end marker + }; + + json _; + CHECK_THROWS_AS(_ = json::from_bson(input), json::parse_error&); } SECTION("Some more complex document") @@ -1178,7 +1248,7 @@ TEST_CASE("BSON document size mismatch") SECTION("top-level document with a negative size") { std::vector const input = {0xFF, 0xFF, 0xFF, 0xFF, 0x00}; - CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 5: syntax error while parsing BSON document: document size -1 does not match the number of bytes read (5)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 4: syntax error while parsing BSON document: document size -1 is less than 5", json::parse_error&); CHECK(json::from_bson(input, true, false).is_discarded()); } @@ -1190,7 +1260,7 @@ TEST_CASE("BSON document size mismatch") // enclosing document instead of "d" std::vector const input = { - 0x00, 0x00, 0x00, 0x00, // outer size + 0x14, 0x00, 0x00, 0x00, // outer size 0x03, 'd', 0x00, // entry: embedded document "d" 0xFF, 0xFF, 0xFF, 0x7F, // embedded size 0x7FFFFFFF 0x0A, 'a', 0x00, // entry: null "a" @@ -1198,7 +1268,24 @@ TEST_CASE("BSON document size mismatch") 0x08, 'h', 0x00, 0x01, // entry: bool "h" = true 0x00 // outer end marker }; - CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 15: syntax error while parsing BSON document: document size 2147483647 does not match the number of bytes read (8)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 11: syntax error while parsing BSON document: document size 2147483647 exceeds the size of the enclosing document", json::parse_error&); + CHECK(json::from_bson(input, true, false).is_discarded()); + } + + SECTION("embedded document whose size is smaller than its content") + { + // {"a":null} is 8 bytes, but the length prefix claims 7 + std::vector const input = + { + 0x14, 0x00, 0x00, 0x00, // outer size + 0x03, 'd', 0x00, // entry: embedded document "d" + 0x07, 0x00, 0x00, 0x00, // embedded size 7 (real is 8) + 0x0A, 'a', 0x00, // entry: null "a" + 0x00, // embedded end marker + 0x08, 'h', 0x00, 0x01, // entry: bool "h" = true + 0x00 // outer end marker + }; + CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 15: syntax error while parsing BSON document: document size 7 does not match the number of bytes read (8)", json::parse_error&); CHECK(json::from_bson(input, true, false).is_discarded()); } @@ -1207,18 +1294,163 @@ TEST_CASE("BSON document size mismatch") // array [42] is 12 bytes, but the length prefix claims 13 std::vector const input = { - 0x00, 0x00, 0x00, 0x00, // outer size + 0x14, 0x00, 0x00, 0x00, // outer size 0x04, 'a', 0x00, // entry: array "a" 0x0D, 0x00, 0x00, 0x00, // array size 13 (real is 12) 0x10, '0', 0x00, 0x2A, 0x00, 0x00, 0x00, // entry: int32 "0" = 42 0x00, // array end marker 0x00 // outer end marker }; - CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 19: syntax error while parsing BSON document: document size 13 does not match the number of bytes read (12)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(input), "[json.exception.parse_error.112] parse error at byte 11: syntax error while parsing BSON document: document size 13 exceeds the size of the enclosing document", json::parse_error&); CHECK(json::from_bson(input, true, false).is_discarded()); } } +TEST_CASE("BSON declared document size bounds the content") +{ + // records the events that a SAX parser receives + struct recorder : nlohmann::json_sax + { + std::string events; + + bool null() override + { + events += "null "; + return true; + } + bool boolean(bool /*val*/) override + { + events += "boolean "; + return true; + } + bool number_integer(json::number_integer_t /*val*/) override + { + events += "integer "; + return true; + } + bool number_unsigned(json::number_unsigned_t /*val*/) override + { + events += "unsigned "; + return true; + } + bool number_float(json::number_float_t /*val*/, const std::string& /*s*/) override + { + events += "float "; + return true; + } + bool string(std::string& /*val*/) override + { + events += "string "; + return true; + } + bool binary(json::binary_t& /*val*/) override + { + events += "binary "; + return true; + } + bool start_object(std::size_t /*elements*/) override + { + events += "{ "; + return true; + } + bool key(std::string& /*val*/) override + { + events += "key "; + return true; + } + bool end_object() override + { + events += "} "; + return true; + } + bool start_array(std::size_t /*elements*/) override + { + events += "[ "; + return true; + } + bool end_array() override + { + events += "] "; + return true; + } + bool parse_error(std::size_t /*position*/, const std::string& /*last_token*/, const json::exception& /*ex*/) override + { + events += "error "; + return false; + } + }; + + SECTION("the content of a too small document is not reported") + { + // {"a":1} is 12 bytes; the length prefix claims 5, so the element + // does not fit + std::vector const input = + { + 0x05, 0x00, 0x00, 0x00, + 0x10, 'a', 0x00, 0x01, 0x00, 0x00, 0x00, + 0x00 + }; + recorder rec; + CHECK(!json::sax_parse(input, &rec, json::input_format_t::bson)); + CHECK(rec.events == "{ error "); + + // also without strict mode + recorder rec2; + CHECK(!json::sax_parse(input, &rec2, json::input_format_t::bson, false)); + CHECK(rec2.events == "{ error "); + } + + SECTION("an embedded document that does not fit its parent is not opened") + { + // the embedded document ends where the enclosing one does, so there + // is no room for the terminator of the enclosing document + std::vector const input = + { + 0x0C, 0x00, 0x00, 0x00, + 0x03, 'd', 0x00, + 0x05, 0x00, 0x00, 0x00, 0x00, + 0x00 + }; + recorder rec; + CHECK(!json::sax_parse(input, &rec, json::input_format_t::bson)); + CHECK(rec.events == "{ key error "); + } + + SECTION("an embedded document that fits exactly") + { + std::vector const input = + { + 0x0D, 0x00, 0x00, 0x00, + 0x03, 'd', 0x00, + 0x05, 0x00, 0x00, 0x00, 0x00, + 0x00 + }; + CHECK(json::from_bson(input) == json({{"d", json::object()}})); + } + + SECTION("a document size that is less than 5") + { + for (const std::int32_t size : + { + 0, 1, 4, -1, (std::numeric_limits::min)() + }) + { + CAPTURE(size) + std::vector input = + { + 0x00, 0x00, 0x00, 0x00, 0x00 + }; + input[0] = static_cast(size & 0xFF); + input[1] = static_cast((size >> 8) & 0xFF); + input[2] = static_cast((size >> 16) & 0xFF); + input[3] = static_cast((size >> 24) & 0xFF); + json _; + CHECK_THROWS_AS(_ = json::from_bson(input), json::parse_error&); + CHECK(json::from_bson(input, true, false).is_discarded()); + } + } +} + TEST_CASE("BSON nesting does not consume the call stack") { // An embedded document or array used to be read by calling back into the @@ -1923,7 +2155,7 @@ TEST_CASE("Invalid document size handling") { std::vector const v = {0x04, 0x00, 0x00, 0x00, 0x00}; json _; - CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 5: syntax error while parsing BSON document: document size 4 does not match the number of bytes read (5)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 4: syntax error while parsing BSON document: document size 4 is less than 5", json::parse_error&); CHECK(json::from_bson(v, true, false).is_discarded()); } @@ -1938,7 +2170,7 @@ TEST_CASE("Invalid document size handling") 0x00 }; json _; - CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 16: syntax error while parsing BSON document: document size 5 does not match the number of bytes read (16)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 5: syntax error while parsing BSON document: document size 5 is too small for its content", json::parse_error&); CHECK(json::from_bson(v, true, false).is_discarded()); } @@ -1969,7 +2201,7 @@ TEST_CASE("Invalid document size handling") 0x00 // object terminator }; json _; - CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 19: syntax error while parsing BSON document: document size 5 does not match the number of bytes read (12)", json::parse_error&); + CHECK_THROWS_WITH_AS(_ = json::from_bson(v), "[json.exception.parse_error.112] parse error at byte 12: syntax error while parsing BSON document: document size 5 is too small for its content", json::parse_error&); CHECK(json::from_bson(v, true, false).is_discarded()); }