UBJSON/BJData reader: reject a container count equal to SIZE_MAX

The reader marks a container whose count is not known by npos, which
is SIZE_MAX. A count of exactly that value was stored unchanged and
then read as "no count", so the BJData input

    [ # M ff ff ff ff ff ff ff ff i 01 ]

parsed as [1] instead of throwing out_of_range.408 as a count of
2^64-2 does. Where size_t is 32 bits, the same holds for 'm' and 'L'
counts of 2^32-1, in UBJSON too.

The counts of 'm', 'M' and the signed types now go through
store_ubjson_count(), which rejects npos with "integer value overflow"
(the message a count beyond size_t already gets), as the CBOR reader
does in get_cbor_container_size(). An ND-array dimension of 2^64-1 gets
the same message now, where it was "excessive ndarray size caused
overflow" (the product check) on 64-bit platforms.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

cmake/test.cmake

@@ -178,10 +178,8 @@ function(_json_test_add_test test_name file main cxx_standard)
             COMMAND ${memcheck_command} $<TARGET_FILE:${test_target}> ${DOCTEST_TEST_FILTER}
             WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
         )
-        # under Valgrind the slowest tests take close to 25 minutes, so ctest's
-        # default timeout of 1500 seconds leaves no headroom
         set_tests_properties(${test_target}_valgrind PROPERTIES
-            LABELS "valgrind" FIXTURES_REQUIRED TEST_DATA TIMEOUT 3600
+            LABELS "valgrind" FIXTURES_REQUIRED TEST_DATA
         )
     endif()
 endfunction()

include/nlohmann/detail/input/binary_reader.hpp

@@ -3123,6 +3123,27 @@ class binary_reader
         return true;
     }
 
+    /*!
+    @brief store the count of an optimized container (or the length of a string)
+
+    A count equal to npos is rejected: npos marks a container whose count is
+    not known, so a count of that value would be read as "no count" (compare
+    get_cbor_container_size()). No container can hold that many elements.
+
+    @param[in]  count   the count read from the input
+    @param[out] result  set to @a count unless it is rejected
+    @return whether @a count is usable
+    */
+    bool store_ubjson_count(const std::size_t count, std::size_t& result)
+    {
+        if (JSON_HEDLEY_UNLIKELY(count == npos))
+        {
+            return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
+        }
+        result = count;
+        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
@@ -3153,8 +3174,7 @@ class binary_reader
         {
             return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
         }
-        result = static_cast<std::size_t>(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char
+        return store_ubjson_count(static_cast<std::size_t>(number), result); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char
-        return true;
     }
 
     /*!
@@ -3231,8 +3251,7 @@ class binary_reader
                 {
                     return false;
                 }
-                result = conditional_static_cast<std::size_t>(number);
+                return store_ubjson_count(conditional_static_cast<std::size_t>(number), result);
-                return true;
             }
 
             case 'M':
@@ -3250,8 +3269,7 @@ class binary_reader
                 {
                     return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
                 }
-                result = detail::conditional_static_cast<std::size_t>(number);
+                return store_ubjson_count(detail::conditional_static_cast<std::size_t>(number), result);
-                return true;
             }
 
             case '[':

include/nlohmann/detail/output/binary_writer.hpp

@@ -2422,6 +2422,7 @@ class binary_writer
             nothing large has been written then, and the caller writes the
             document in two passes, which reports that error
     */
+    template<typename SinkType = OutputSinkType, typename std::enable_if<is_detected<sink_written_t, SinkType>::value, int>::type = 0>
     bool write_bson_document_single_pass(const BasicJsonType& document)
     {
         const std::size_t start = oa.written();

single_include/nlohmann/json.hpp

@@ -21130,6 +21130,27 @@ class binary_reader
         return true;
     }
 
+    /*!
+    @brief store the count of an optimized container (or the length of a string)
+
+    A count equal to npos is rejected: npos marks a container whose count is
+    not known, so a count of that value would be read as "no count" (compare
+    get_cbor_container_size()). No container can hold that many elements.
+
+    @param[in]  count   the count read from the input
+    @param[out] result  set to @a count unless it is rejected
+    @return whether @a count is usable
+    */
+    bool store_ubjson_count(const std::size_t count, std::size_t& result)
+    {
+        if (JSON_HEDLEY_UNLIKELY(count == npos))
+        {
+            return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
+        }
+        result = count;
+        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
@@ -21160,8 +21181,7 @@ class binary_reader
         {
             return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
         }
-        result = static_cast<std::size_t>(number); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char
+        return store_ubjson_count(static_cast<std::size_t>(number), result); // NOLINT(bugprone-signed-char-misuse,cert-str34-c): number is not a char
-        return true;
     }
 
     /*!
@@ -21238,8 +21258,7 @@ class binary_reader
                 {
                     return false;
                 }
-                result = conditional_static_cast<std::size_t>(number);
+                return store_ubjson_count(conditional_static_cast<std::size_t>(number), result);
-                return true;
             }
 
             case 'M':
@@ -21257,8 +21276,7 @@ class binary_reader
                 {
                     return out_of_range_error(exception_id::container_too_large, "integer value overflow", "size");
                 }
-                result = detail::conditional_static_cast<std::size_t>(number);
+                return store_ubjson_count(detail::conditional_static_cast<std::size_t>(number), result);
-                return true;
             }
 
             case '[':
@@ -30394,6 +30412,7 @@ class binary_writer
             nothing large has been written then, and the caller writes the
             document in two passes, which reports that error
     */
+    template<typename SinkType = OutputSinkType, typename std::enable_if<is_detected<sink_written_t, SinkType>::value, int>::type = 0>
     bool write_bson_document_single_pass(const BasicJsonType& document)
     {
         const std::size_t start = oa.written();

tests/src/unit-bjdata.cpp

@@ -3211,14 +3211,34 @@ TEST_CASE("BJData")
 #endif
                 CHECK(json::from_bjdata(vM, true, false).is_discarded());
 
-#if SIZE_MAX != 0xffffffff
+                // the first dimension is 2^64-1, which is rejected as a count (see below)
-                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vMX), "[json.exception.out_of_range.408] syntax error while parsing BJData size: excessive ndarray size caused overflow", json::out_of_range&);
-#else
                 CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vMX), "[json.exception.out_of_range.408] syntax error while parsing BJData size: integer value overflow", json::out_of_range&);
-#endif
                 CHECK(json::from_bjdata(vMX, true, false).is_discarded());
             }
 
+            SECTION("optimized array: count equal to SIZE_MAX")
+            {
+                // the reader marks a container without a count by SIZE_MAX, so
+                // a count of that value must not be read as "no count"
+                // (it used to parse as [1])
+                json _;
+#if SIZE_MAX != 0xffffffff
+                std::vector<uint8_t> const vM = {'[', '#', 'M', 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 'i', 0x01, ']'};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vM), "[json.exception.out_of_range.408] syntax error while parsing BJData size: integer value overflow", json::out_of_range&);
+                CHECK(json::from_bjdata(vM, true, false).is_discarded());
+                // one less is an ordinary excessive count
+                std::vector<uint8_t> const vM1 = {'[', '#', 'M', 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 'i', 0x01, ']'};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vM1), "[json.exception.out_of_range.408] excessive array size: 18446744073709551614", json::out_of_range&);
+#else
+                std::vector<uint8_t> const vm = {'[', '#', 'm', 0xFF, 0xFF, 0xFF, 0xFF, 'i', 0x01, ']'};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vm), "[json.exception.out_of_range.408] syntax error while parsing BJData size: integer value overflow", json::out_of_range&);
+                CHECK(json::from_bjdata(vm, true, false).is_discarded());
+                std::vector<uint8_t> const vL = {'[', '#', 'L', 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00, 'i', 0x01, ']'};
+                CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vL), "[json.exception.out_of_range.408] syntax error while parsing BJData size: integer value overflow", json::out_of_range&);
+                CHECK(json::from_bjdata(vL, true, false).is_discarded());
+#endif
+            }
+
             SECTION("optimized array: integer value overflow")
             {
 #if SIZE_MAX == 0xffffffff

tests/src/unit-ubjson.cpp

@@ -2360,6 +2360,18 @@ TEST_CASE("UBJSON optimized arrays of a valueless type are bounded")
         }
     }
 
+    SECTION("a count equal to SIZE_MAX is rejected")
+    {
+        // the reader marks a container without a count by SIZE_MAX, so a
+        // count of that value must not be read as "no count"; UBJSON's
+        // largest count type (int64) can only reach it where size_t is 32 bits
+#if SIZE_MAX == 0xffffffff
+        const std::vector<uint8_t> input = {'[', '#', 'L', 0x00, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 'i', 0x01, ']'};
+        CHECK_THROWS_WITH_AS(_ = json::from_ubjson(input), "[json.exception.out_of_range.408] syntax error while parsing UBJSON size: integer value overflow", json::out_of_range&);
+        CHECK(json::from_ubjson(input, true, false).is_discarded());
+#endif
+    }
+
     SECTION("ordinary counts are unaffected")
     {
         CHECK(json::from_ubjson(std::vector<uint8_t>({'[', '$', 'Z', '#', 'i', 3})) == json({nullptr, nullptr, nullptr}));