Summary
The BSON parser reads the mandatory leading document_size field but never validates or otherwise uses it. This means structurally malformed BSON documents (including ones with a negative or nonsensical size header) are silently accepted rather than rejected.
Affected code
include/nlohmann/detail/input/binary_reader.hpp:
bool parse_bson_internal()
{
std::int32_t document_size{};
get_number<std::int32_t, true>(input_format_t::bson, document_size);
// document_size is never checked (not > 0, not compared against
// remaining input) and is never referenced again in this function.
...
}
The same pattern exists in parse_bson_array().
Element-list parsing (parse_bson_element_list) terminates purely by encountering a 0x00 element-type byte in the stream — it does not use document_size as a bound at all. So today document_size is effectively dead code beyond consuming 4 bytes of input.
Impact
Note: I initially assumed this could cause a loop-termination bug or signed-integer-overflow-driven memory issue, since that's the classic failure mode for an unchecked size header. That is not accurate for the current implementation — document_size isn't used in any arithmetic or comparison, so there's no active memory-safety path here.
The real impact is narrower: the parser doesn't enforce the BSON spec's minimum document size (5 bytes: 4-byte length + terminating 0x00) or that the declared size is internally consistent with the actual content, so malformed/garbage BSON (e.g. a document size of -1, 0, or a value wildly inconsistent with the real content) is processed as if valid instead of being rejected up front with a clear parse error. This is a correctness/spec-compliance gap and a defense-in-depth issue rather than an exploitable DoS/memory-corruption bug as currently written — but it should still be closed off, both for spec correctness and in case document_size is used for bounds-checking in a future refactor (where an unchecked negative value would become load-bearing).
Proof of concept
#include <nlohmann/json.hpp>
int main()
{
// document_size = -1 (0xFFFFFFFF, little-endian), followed by an
// immediate terminator. Spec requires a positive size >= 5.
std::vector<std::uint8_t> data = {0xFF, 0xFF, 0xFF, 0xFF, 0x00};
auto j = nlohmann::json::from_bson(data); // currently accepted
}
Suggested fix
Validate document_size immediately after reading it in both parse_bson_internal() and parse_bson_array():
if (document_size < 5)
{
return sax->parse_error(chars_read, get_token_string(),
parse_error::create(112, chars_read,
exception_message(input_format_t::bson, "invalid document_size: " + std::to_string(document_size), "value"), nullptr));
}
Optionally, also verify document_size is consistent with the number of bytes actually available/consumed for the document.
Environment
- nlohmann/json version:
develop branch (current)
Summary
The BSON parser reads the mandatory leading
document_sizefield but never validates or otherwise uses it. This means structurally malformed BSON documents (including ones with a negative or nonsensical size header) are silently accepted rather than rejected.Affected code
include/nlohmann/detail/input/binary_reader.hpp:The same pattern exists in
parse_bson_array().Element-list parsing (
parse_bson_element_list) terminates purely by encountering a0x00element-type byte in the stream — it does not usedocument_sizeas a bound at all. So todaydocument_sizeis effectively dead code beyond consuming 4 bytes of input.Impact
Note: I initially assumed this could cause a loop-termination bug or signed-integer-overflow-driven memory issue, since that's the classic failure mode for an unchecked size header. That is not accurate for the current implementation —
document_sizeisn't used in any arithmetic or comparison, so there's no active memory-safety path here.The real impact is narrower: the parser doesn't enforce the BSON spec's minimum document size (5 bytes: 4-byte length + terminating
0x00) or that the declared size is internally consistent with the actual content, so malformed/garbage BSON (e.g. a document size of-1,0, or a value wildly inconsistent with the real content) is processed as if valid instead of being rejected up front with a clear parse error. This is a correctness/spec-compliance gap and a defense-in-depth issue rather than an exploitable DoS/memory-corruption bug as currently written — but it should still be closed off, both for spec correctness and in casedocument_sizeis used for bounds-checking in a future refactor (where an unchecked negative value would become load-bearing).Proof of concept
Suggested fix
Validate
document_sizeimmediately after reading it in bothparse_bson_internal()andparse_bson_array():Optionally, also verify
document_sizeis consistent with the number of bytes actually available/consumed for the document.Environment
developbranch (current)