From 2610d176efd55b835acfc2c8a17440a20e14ee4e Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 30 Sep 2026 18:56:25 +0200 Subject: [PATCH] Deduplicate UBJSON/BJData signed-count handling, drop dead ndarray checks get_ubjson_size_value()'s 'i'/'I'/'l'/'L' cases each read a differently sized signed integer and then repeated the same "reject negative with error 113" check; only 'L' additionally checked value_in_range_of for the out_of_range.408 case. Any change to that error path had to be made four times. Add get_ubjson_signed_count(std::size_t&), doing the read, the negative check and the range check once, and route all four markers through it. The range check is a no-op for 'i'/'I'/'l' (their values always fit std::size_t) and only live for 'L' on a 32-bit std::size_t target, matching today's behavior exactly. In the ndarray dimension-product loop, the preceding loop already returns early on any zero dimension and result starts at 1, so `i > 0` in the pre-multiplication overflow check was always true, and `result == 0` in the post-multiplication check could not be reached either: two positive factors whose product does not overflow (as the pre-check already guarantees) cannot be zero. Drop the dead `i > 0 &&` and narrow the post-check to `result == npos`, the one case the pre-check cannot rule out (an exact, non-overflowing match with the sentinel reserved for unknown-size containers), with a comment explaining why. Verified byte-for-byte identical behavior before/after with a standalone probe covering negative counts for every marker, a matching positive count, and ndarray inputs, plus the full unit-ubjson and unit-bjdata suites (same assertion counts as before this change). Overlaps #5601 (rewrites the four parse_error calls and the overflow checks touched here) and #5607/#5707 (touch neighboring lines in the same functions). #5711 item 6 Signed-off-by: Niels Lohmann --- .../nlohmann/detail/input/binary_reader.hpp | 120 ++++++++---------- single_include/nlohmann/json.hpp | 120 ++++++++---------- 2 files changed, 106 insertions(+), 134 deletions(-) diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 33dcf6bd0..e1fc67a77 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -2668,6 +2668,42 @@ class binary_reader return true; } + /*! + @brief read a UBJSON/BJData optimized-container count of a signed marker + type ('i', 'I', 'l', 'L') and narrow it to std::size_t + + Every signed count marker rejects a negative value the same way (error + 113); the value_in_range_of check additionally needed for 'L' is only + ever live when @a SignedType is std::int64_t on a target where + std::size_t is narrower (e.g. 32-bit), since 'i'/'I'/'l' can never exceed + std::size_t there. + + @tparam SignedType std::int8_t, std::int16_t, std::int32_t or std::int64_t + @param[out] result the count narrowed to std::size_t + @return whether reading and validating succeeded + */ + template + bool get_ubjson_signed_count(std::size_t& result) + { + SignedType number{}; + if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) + { + return false; + } + if (JSON_HEDLEY_UNLIKELY(number < 0)) + { + return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, + exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); + } + if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(number))) + { + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, + exception_message(input_format, "integer value overflow", "size"), nullptr)); + } + result = static_cast(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char + return true; + } + /*! @param[out] result determined size @param[in,out] is_ndarray for input, `true` means already inside an ndarray vector @@ -2700,73 +2736,16 @@ class binary_reader } case 'i': - { - std::int8_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char - return true; - } + return get_ubjson_signed_count(result); case 'I': - { - std::int16_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'l': - { - std::int32_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'L': - { - std::int64_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - if (!value_in_range_of(number)) - { - return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, - exception_message(input_format, "integer value overflow", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'u': { @@ -2857,16 +2836,23 @@ class binary_reader result = 1; for (auto i : dim) { - // Pre-multiplication overflow check: if i > 0 and result > SIZE_MAX/i, then result*i would overflow. - // This check must happen before multiplication since overflow detection after the fact is unreliable - // as modular arithmetic can produce any value, not just 0 or SIZE_MAX. - if (JSON_HEDLEY_UNLIKELY(i > 0 && result > (std::numeric_limits::max)() / i)) + // Pre-multiplication overflow check: since the loop above + // already rejected any zero dimension, i is always > 0 + // here, so result > SIZE_MAX/i means result*i would + // overflow. This check must happen before multiplication + // since overflow detection after the fact is unreliable, + // as modular arithmetic can produce any value, not just 0 + // or SIZE_MAX. + if (JSON_HEDLEY_UNLIKELY(result > (std::numeric_limits::max)() / i)) { return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, exception_message(input_format, "excessive ndarray size caused overflow", "size"), nullptr)); } result *= i; - // Additional post-multiplication check to catch any edge cases the pre-check might miss - if (result == 0 || result == npos) + // the pre-check above already rules out result becoming 0 + // by overflow; the only value it cannot rule out is an + // exact match with npos, the sentinel reserved for an + // unknown-size container (see get_ubjson_size_type()) + if (result == npos) { return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, exception_message(input_format, "excessive ndarray size caused overflow", "size"), nullptr)); } diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index c3ad969c3..0fe45f561 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -15428,6 +15428,42 @@ class binary_reader return true; } + /*! + @brief read a UBJSON/BJData optimized-container count of a signed marker + type ('i', 'I', 'l', 'L') and narrow it to std::size_t + + Every signed count marker rejects a negative value the same way (error + 113); the value_in_range_of check additionally needed for 'L' is only + ever live when @a SignedType is std::int64_t on a target where + std::size_t is narrower (e.g. 32-bit), since 'i'/'I'/'l' can never exceed + std::size_t there. + + @tparam SignedType std::int8_t, std::int16_t, std::int32_t or std::int64_t + @param[out] result the count narrowed to std::size_t + @return whether reading and validating succeeded + */ + template + bool get_ubjson_signed_count(std::size_t& result) + { + SignedType number{}; + if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) + { + return false; + } + if (JSON_HEDLEY_UNLIKELY(number < 0)) + { + return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, + exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); + } + if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(number))) + { + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, + exception_message(input_format, "integer value overflow", "size"), nullptr)); + } + result = static_cast(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char + return true; + } + /*! @param[out] result determined size @param[in,out] is_ndarray for input, `true` means already inside an ndarray vector @@ -15460,73 +15496,16 @@ class binary_reader } case 'i': - { - std::int8_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char - return true; - } + return get_ubjson_signed_count(result); case 'I': - { - std::int16_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'l': - { - std::int32_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'L': - { - std::int64_t number{}; - if (JSON_HEDLEY_UNLIKELY(!get_number(input_format, number))) - { - return false; - } - if (number < 0) - { - return sax->parse_error(chars_read, get_token_string(), parse_error::create(113, chars_read, - exception_message(input_format, "count in an optimized container must be positive", "size"), nullptr)); - } - if (!value_in_range_of(number)) - { - return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, - exception_message(input_format, "integer value overflow", "size"), nullptr)); - } - result = static_cast(number); - return true; - } + return get_ubjson_signed_count(result); case 'u': { @@ -15617,16 +15596,23 @@ class binary_reader result = 1; for (auto i : dim) { - // Pre-multiplication overflow check: if i > 0 and result > SIZE_MAX/i, then result*i would overflow. - // This check must happen before multiplication since overflow detection after the fact is unreliable - // as modular arithmetic can produce any value, not just 0 or SIZE_MAX. - if (JSON_HEDLEY_UNLIKELY(i > 0 && result > (std::numeric_limits::max)() / i)) + // Pre-multiplication overflow check: since the loop above + // already rejected any zero dimension, i is always > 0 + // here, so result > SIZE_MAX/i means result*i would + // overflow. This check must happen before multiplication + // since overflow detection after the fact is unreliable, + // as modular arithmetic can produce any value, not just 0 + // or SIZE_MAX. + if (JSON_HEDLEY_UNLIKELY(result > (std::numeric_limits::max)() / i)) { return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, exception_message(input_format, "excessive ndarray size caused overflow", "size"), nullptr)); } result *= i; - // Additional post-multiplication check to catch any edge cases the pre-check might miss - if (result == 0 || result == npos) + // the pre-check above already rules out result becoming 0 + // by overflow; the only value it cannot rule out is an + // exact match with npos, the sentinel reserved for an + // unknown-size container (see get_ubjson_size_type()) + if (result == npos) { return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, exception_message(input_format, "excessive ndarray size caused overflow", "size"), nullptr)); }