Repository navigation
BSON reader ignores get_number() return values and skips an EOF check for booleans #5309
Description
Description
Three get_number() calls in the BSON reader discard their bool result, and the boolean element handler never checks for EOF:
| location | issue |
|---|---|
include/nlohmann/detail/input/binary_reader.hpp:200 |
parse_bson_internal — document size prefix, return value ignored |
binary_reader.hpp:434 |
parse_bson_array — document size prefix, return value ignored |
binary_reader.hpp:290 |
get_bson_binary — binary subtype, return value ignored |
binary_reader.hpp:343 |
case 0x08: sax->boolean(get() != 0) — no EOF check, so EOF yields boolean(true) |
Expected vs. actual results
I was not able to turn any of these into an accepted malformed document. Every truncation I tried still produced a correct parse_error.110, because a later check always catches the failure:
--- empty input: EXC parse_error.110 at byte 1 (BSON number: unexpected end of input)
--- 2-byte input: EXC parse_error.110 at byte 3 (BSON number: unexpected end of input)
--- 4-byte input (size only): EXC parse_error.110 at byte 5 (BSON element list: unexpected end of input)
--- binary len=0 truncated at subtype: EXC parse_error.110 at byte 12 (BSON number: unexpected end of input)
--- boolean truncated at EOF: EXC parse_error.110 at byte 9 (BSON element list: unexpected end of input)
So this is a robustness/maintainability issue rather than a live bug — but it is correctness by accident, and it is fragile. The get_bson_binary case in particular is only safe today because of the document-size check added in #5287, which did not exist when that line was written: with a declared binary length of 0 and input truncated exactly at the subtype byte, get_binary() succeeds trivially and an empty binary with an uninitialised subtype is handed to the SAX parser; only the enclosing size/terminator check turns that into an error.
Similarly, the boolean path emits boolean(true) for an EOF byte before the error surfaces on the next loop iteration, which is inconsistent with how every other element type is handled.
Suggested fix
Check the return value of the three get_number() calls and short-circuit on failure, and give case 0x08 an explicit unexpect_eof() check (or route it through get_number<std::uint8_t>) so it does not depend on downstream validation.
Compiler and operating system
g++ 13.3.0 (Ubuntu 24.04), -std=c++11 -O2, Linux x86-64
Library version
develop @ 06ac77f
Activity