From 48a102c2c5ef7c2552ea291f5527a8481c98a7a7 Mon Sep 17 00:00:00 2001 From: Florian Albrechtskirchinger Date: Thu, 9 Jun 2022 08:22:58 +0200 Subject: [PATCH] Fix ndarray dimension signedness, fix ndarray length overflow (2); add 32bit unit test (#3523) * Fix ndarray dimension signness, fix ndarray length overflow, close #3519 * detect size overflow in ubjson and bjdata * force reformatting * Fix MSVC compiler warning * Add value_in_range_of trait * Use value_in_range_of trait * Correct 408 parse_errors to out_of_range * Add 32bit unit test The test can be enabled by setting JSON_32bitTest=ON. * Exclude unreachable lines from coverage Certain lines are unreachable in 64bit builds. Co-authored-by: Qianqian Fang --- .../nlohmann/detail/input/binary_reader.hpp | 18 ++- include/nlohmann/detail/meta/type_traits.hpp | 95 ++++++++++++++ single_include/nlohmann/json.hpp | 113 ++++++++++++++++- tests/CMakeLists.txt | 68 ++++++---- tests/src/unit-32bit.cpp | 117 +++++++++++++++++ tests/src/unit-bjdata.cpp | 120 +++++++++++++++++- 6 files changed, 503 insertions(+), 28 deletions(-) create mode 100644 tests/src/unit-32bit.cpp diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index cbcb41588..9332af3e6 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -2079,6 +2079,12 @@ class binary_reader 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)) + { + // undo coverage exclusion once the 32bit test is run as part of CI (#3524) + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, // LCOV_EXCL_LINE + exception_message(input_format, "integer value overflow", "size"), nullptr)); // LCOV_EXCL_LINE + } result = static_cast(number); return true; } @@ -2124,6 +2130,12 @@ class binary_reader { return false; } + if (!value_in_range_of(number)) + { + // undo coverage exclusion once the 32bit test is run as part of CI (#3524) + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, // LCOV_EXCL_LINE + exception_message(input_format, "integer value overflow", "size"), nullptr)); // LCOV_EXCL_LINE + } result = detail::conditional_static_cast(number); return true; } @@ -2168,7 +2180,11 @@ class binary_reader for (auto i : dim) { result *= i; - if (JSON_HEDLEY_UNLIKELY(!sax->number_integer(static_cast(i)))) + if (result == 0) // because dim elements shall not have zeros, result = 0 means overflow happened + { + 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)); + } + if (JSON_HEDLEY_UNLIKELY(!sax->number_unsigned(static_cast(i)))) { return false; } diff --git a/include/nlohmann/detail/meta/type_traits.hpp b/include/nlohmann/detail/meta/type_traits.hpp index 5e3ea3737..0901b71bb 100644 --- a/include/nlohmann/detail/meta/type_traits.hpp +++ b/include/nlohmann/detail/meta/type_traits.hpp @@ -577,5 +577,100 @@ T conditional_static_cast(U value) return value; } +template +using all_integral = conjunction...>; + +template +using all_signed = conjunction...>; + +template +using all_unsigned = conjunction...>; + +// there's a disjunction trait in another PR; replace when merged +template +using same_sign = std::integral_constant < bool, + all_signed::value || all_unsigned::value >; + +template +using never_out_of_range = std::integral_constant < bool, + (std::is_signed::value && (sizeof(T) < sizeof(OfType))) + || (same_sign::value && sizeof(OfType) == sizeof(T)) >; + +template::value, + bool TSigned = std::is_signed::value> +struct value_in_range_of_impl2; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return val >= 0 && static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) >= static_cast(std::numeric_limits::min()) + && static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template::value, + typename = detail::enable_if_t::value>> +struct value_in_range_of_impl1; + +template +struct value_in_range_of_impl1 +{ + static constexpr bool test(T val) + { + return value_in_range_of_impl2::test(val); + } +}; + +template +struct value_in_range_of_impl1 +{ + static constexpr bool test(T /*val*/) + { + return true; + } +}; + +template +inline constexpr bool value_in_range_of(T val) +{ + return value_in_range_of_impl1::test(val); +} + } // namespace detail } // namespace nlohmann diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 2837e74b9..3dcd48027 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -3768,6 +3768,101 @@ T conditional_static_cast(U value) return value; } +template +using all_integral = conjunction...>; + +template +using all_signed = conjunction...>; + +template +using all_unsigned = conjunction...>; + +// there's a disjunction trait in another PR; replace when merged +template +using same_sign = std::integral_constant < bool, + all_signed::value || all_unsigned::value >; + +template +using never_out_of_range = std::integral_constant < bool, + (std::is_signed::value && (sizeof(T) < sizeof(OfType))) + || (same_sign::value && sizeof(OfType) == sizeof(T)) >; + +template::value, + bool TSigned = std::is_signed::value> +struct value_in_range_of_impl2; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return val >= 0 && static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + + +template +struct value_in_range_of_impl2 +{ + static constexpr bool test(T val) + { + using CommonType = typename std::common_type::type; + return static_cast(val) >= static_cast(std::numeric_limits::min()) + && static_cast(val) <= static_cast(std::numeric_limits::max()); + } +}; + +template::value, + typename = detail::enable_if_t::value>> +struct value_in_range_of_impl1; + +template +struct value_in_range_of_impl1 +{ + static constexpr bool test(T val) + { + return value_in_range_of_impl2::test(val); + } +}; + +template +struct value_in_range_of_impl1 +{ + static constexpr bool test(T /*val*/) + { + return true; + } +}; + +template +inline constexpr bool value_in_range_of(T val) +{ + return value_in_range_of_impl1::test(val); +} + } // namespace detail } // namespace nlohmann @@ -10669,6 +10764,12 @@ class binary_reader 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)) + { + // undo coverage exclusion once the 32bit test is run as part of CI (#3524) + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, // LCOV_EXCL_LINE + exception_message(input_format, "integer value overflow", "size"), nullptr)); // LCOV_EXCL_LINE + } result = static_cast(number); return true; } @@ -10714,6 +10815,12 @@ class binary_reader { return false; } + if (!value_in_range_of(number)) + { + // undo coverage exclusion once the 32bit test is run as part of CI (#3524) + return sax->parse_error(chars_read, get_token_string(), out_of_range::create(408, // LCOV_EXCL_LINE + exception_message(input_format, "integer value overflow", "size"), nullptr)); // LCOV_EXCL_LINE + } result = detail::conditional_static_cast(number); return true; } @@ -10758,7 +10865,11 @@ class binary_reader for (auto i : dim) { result *= i; - if (JSON_HEDLEY_UNLIKELY(!sax->number_integer(static_cast(i)))) + if (result == 0) // because dim elements shall not have zeros, result = 0 means overflow happened + { + 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)); + } + if (JSON_HEDLEY_UNLIKELY(!sax->number_unsigned(static_cast(i)))) { return false; } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 010c4950e..ef87c4909 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -2,6 +2,7 @@ cmake_minimum_required(VERSION 3.13) option(JSON_Valgrind "Execute test suite with Valgrind." OFF) option(JSON_FastTests "Skip expensive/slow tests." OFF) +option(JSON_32bitTest "Enable the 32bit unit test." OFF) set(JSON_TestStandards "" CACHE STRING "The list of standards to test explicitly.") @@ -33,35 +34,40 @@ endif() # test_main library with shared code to speed up build and common settings ############################################################################# -add_library(test_main OBJECT src/unit.cpp) -target_compile_definitions(test_main PUBLIC +set(test_main_SOURCES src/unit.cpp) +set(test_main_COMPILE_DEFINITIONS PUBLIC DOCTEST_CONFIG_SUPER_FAST_ASSERTS - JSON_TEST_KEEP_MACROS -) -target_compile_features(test_main PRIVATE cxx_std_11) -target_compile_options(test_main PUBLIC - $<$:/EHsc;$<$:/Od>> - # MSVC: Force to always compile with W4 - # Disable warning C4566: character represented by universal-character-name '\uFF01' - # cannot be represented in the current code page (1252) - # Disable warning C4996: 'nlohmann::basic_json<...>::operator <<': was declared deprecated - $<$:/W4 /wd4566 /wd4996> - # https://github.com/nlohmann/json/issues/1114 - $<$:/bigobj> $<$:-Wa,-mbig-obj> + JSON_TEST_KEEP_MACROS) +set(test_main_COMPILE_FEATURES PRIVATE cxx_std_11) +set(test_main_COMPILE_OPTIONS + PUBLIC + $<$:/EHsc;$<$:/Od>> + # MSVC: Force to always compile with W4 + # Disable warning C4566: character represented by universal-character-name '\uFF01' + # cannot be represented in the current code page (1252) + # Disable warning C4996: 'nlohmann::basic_json<...>::operator <<': was declared deprecated + $<$:/W4 /wd4566 /wd4996> + # https://github.com/nlohmann/json/issues/1114 + $<$:/bigobj> $<$:-Wa,-mbig-obj> - # https://github.com/nlohmann/json/pull/3229 - $<$:-diag-disable=2196> + # https://github.com/nlohmann/json/pull/3229 + $<$:-diag-disable=2196> - $<$>:-Wno-deprecated;-Wno-float-equal> - $<$:-Wno-deprecated-declarations> - $<$:-diag-disable=1786> -) -target_include_directories(test_main PUBLIC + $<$>:-Wno-deprecated;-Wno-float-equal> + $<$:-Wno-deprecated-declarations> + $<$:-diag-disable=1786>) +set(test_main_INCLUDE_DIRECTORIES PUBLIC thirdparty/doctest thirdparty/fifo_map - ${PROJECT_BINARY_DIR}/include -) -target_link_libraries(test_main PUBLIC ${NLOHMANN_JSON_TARGET_NAME}) + ${PROJECT_BINARY_DIR}/include) +set(test_main_LINK_LIBRARIES PUBLIC ${NLOHMANN_JSON_TARGET_NAME}) + +add_library(test_main OBJECT ${test_main_SOURCES}) +target_compile_definitions(test_main ${test_main_COMPILE_DEFINITIONS}) +target_compile_features(test_main ${test_main_COMPILE_FEATURES}) +target_compile_options(test_main ${test_main_COMPILE_OPTIONS}) +target_include_directories(test_main ${test_main_INCLUDE_DIRECTORIES}) +target_link_libraries(test_main ${test_main_LINK_LIBRARIES}) ############################################################################# # define test- and standard-specific build settings @@ -119,10 +125,24 @@ message(STATUS "${msg}") # *DO* use json_test_set_test_options() above this line file(GLOB files src/unit-*.cpp) +list(FILTER files EXCLUDE REGEX "src/unit-32bit.cpp") foreach(file ${files}) json_test_add_test_for(${file} MAIN test_main CXX_STANDARDS ${test_cxx_standards} ${test_force}) endforeach() +if(JSON_32bitTest) + add_library(test_main32 OBJECT ${test_main_SOURCES}) + target_compile_definitions(test_main32 ${test_main_COMPILE_DEFINITIONS}) + target_compile_features(test_main32 ${test_main_COMPILE_FEATURES}) + target_compile_options(test_main32 ${test_main_COMPILE_OPTIONS} -m32) + target_include_directories(test_main32 ${test_main_INCLUDE_DIRECTORIES}) + target_link_libraries(test_main32 ${test_main_LINK_LIBRARIES}) + target_link_options(test_main32 PUBLIC -m32) + + json_test_add_test_for("src/unit-32bit.cpp" MAIN test_main32 + CXX_STANDARDS ${test_cxx_standards} ${test_force}) +endif() + # test legacy comparison of discarded values json_test_set_test_options(test-comparison_legacy COMPILE_DEFINITIONS JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON=1 diff --git a/tests/src/unit-32bit.cpp b/tests/src/unit-32bit.cpp new file mode 100644 index 000000000..711dda5a0 --- /dev/null +++ b/tests/src/unit-32bit.cpp @@ -0,0 +1,117 @@ +#include "doctest_compatibility.h" + +#include +using nlohmann::json; + +#include // SIZE_MAX +#include // numeric_limits + + +template +struct trait_test_arg +{ + using of_type = OfType; + using type = T; + static constexpr bool min_in_range = MinInRange; + static constexpr bool max_in_range = MaxInRange; +}; + +TEST_CASE_TEMPLATE_DEFINE("value_in_range_of trait", T, value_in_range_of_test) +{ + using nlohmann::detail::value_in_range_of; + + using of_type = typename T::of_type; + using type = typename T::type; + constexpr bool min_in_range = T::min_in_range; + constexpr bool max_in_range = T::max_in_range; + + type val_min = std::numeric_limits::min(); + type val_min2 = val_min + 1; + type val_max = std::numeric_limits::max(); + type val_max2 = val_max - 1; + + REQUIRE(CHAR_BIT == 8); + + std::string of_type_str; + if (std::is_unsigned::value) + { + of_type_str += "u"; + } + of_type_str += "int"; + of_type_str += std::to_string(sizeof(of_type) * 8); + + INFO("of_type := ", of_type_str); + + std::string type_str; + if (std::is_unsigned::value) + { + type_str += "u"; + } + type_str += "int"; + type_str += std::to_string(sizeof(type) * 8); + + INFO("type := ", type_str); + + CAPTURE(val_min); + CAPTURE(min_in_range); + CAPTURE(val_max); + CAPTURE(max_in_range); + + if (min_in_range) + { + CHECK(value_in_range_of(val_min)); + CHECK(value_in_range_of(val_min2)); + } + else + { + CHECK_FALSE(value_in_range_of(val_min)); + CHECK_FALSE(value_in_range_of(val_min2)); + } + + if (max_in_range) + { + CHECK(value_in_range_of(val_max)); + CHECK(value_in_range_of(val_max2)); + } + else + { + CHECK_FALSE(value_in_range_of(val_max)); + CHECK_FALSE(value_in_range_of(val_max2)); + } +} + + +TEST_CASE("32bit") +{ + REQUIRE(SIZE_MAX == 0xffffffff); +} + +TEST_CASE_TEMPLATE_INVOKE(value_in_range_of_test, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg); + +TEST_CASE("BJData") +{ + SECTION("parse errors") + { + SECTION("array") + { + SECTION("optimized array: no size following type") + { + std::vector v = {'[', '$', 'i', 2}; + json _; + CHECK_THROWS_WITH_AS(_ = json::from_bjdata(v), "[json.exception.parse_error.112] parse error at byte 4: syntax error while parsing BJData size: expected '#' after type information; last byte: 0x02", json::parse_error&); + } + + SECTION("optimized array: negative size") + { + std::vector vM = {'[', '$', 'M', '#', '[', 'I', 0x00, 0x20, 'M', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xFF, ']'}; + + json _; + 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&); + } + } + } +} diff --git a/tests/src/unit-bjdata.cpp b/tests/src/unit-bjdata.cpp index a2ea7820f..974ee3a0a 100644 --- a/tests/src/unit-bjdata.cpp +++ b/tests/src/unit-bjdata.cpp @@ -29,6 +29,8 @@ SOFTWARE. #include "doctest_compatibility.h" +#include +#include #include using nlohmann::json; @@ -116,6 +118,112 @@ class SaxCountdown }; } // namespace +// at some point in the future, a unit test dedicated to type traits might be a good idea +template +struct trait_test_arg +{ + using of_type = OfType; + using type = T; + static constexpr bool min_in_range = MinInRange; + static constexpr bool max_in_range = MaxInRange; +}; + +TEST_CASE_TEMPLATE_DEFINE("value_in_range_of trait", T, value_in_range_of_test) +{ + using nlohmann::detail::value_in_range_of; + + using of_type = typename T::of_type; + using type = typename T::type; + constexpr bool min_in_range = T::min_in_range; + constexpr bool max_in_range = T::max_in_range; + + type val_min = std::numeric_limits::min(); + type val_min2 = val_min + 1; + type val_max = std::numeric_limits::max(); + type val_max2 = val_max - 1; + + REQUIRE(CHAR_BIT == 8); + + std::string of_type_str; + if (std::is_unsigned::value) + { + of_type_str += "u"; + } + of_type_str += "int"; + of_type_str += std::to_string(sizeof(of_type) * 8); + + INFO("of_type := ", of_type_str); + + std::string type_str; + if (std::is_unsigned::value) + { + type_str += "u"; + } + type_str += "int"; + type_str += std::to_string(sizeof(type) * 8); + + INFO("type := ", type_str); + + CAPTURE(val_min); + CAPTURE(min_in_range); + CAPTURE(val_max); + CAPTURE(max_in_range); + + if (min_in_range) + { + CHECK(value_in_range_of(val_min)); + CHECK(value_in_range_of(val_min2)); + } + else + { + CHECK_FALSE(value_in_range_of(val_min)); + CHECK_FALSE(value_in_range_of(val_min2)); + } + + if (max_in_range) + { + CHECK(value_in_range_of(val_max)); + CHECK(value_in_range_of(val_max2)); + } + else + { + CHECK_FALSE(value_in_range_of(val_max)); + CHECK_FALSE(value_in_range_of(val_max2)); + } +} + +TEST_CASE_TEMPLATE_INVOKE(value_in_range_of_test, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg); + +#if SIZE_MAX == 0xffffffff +TEST_CASE_TEMPLATE_INVOKE(value_in_range_of_test, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg); +#else +TEST_CASE_TEMPLATE_INVOKE(value_in_range_of_test, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg, \ + trait_test_arg); +#endif + TEST_CASE("BJData") { SECTION("individual values") @@ -2511,6 +2619,7 @@ TEST_CASE("BJData") std::vector vI = {'[', '#', 'I', 0x00, 0xF1}; std::vector vl = {'[', '#', 'l', 0x00, 0x00, 0x00, 0xF2}; std::vector vL = {'[', '#', 'L', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xF3}; + std::vector vM = {'[', '$', 'M', '#', '[', 'I', 0x00, 0x20, 'M', 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0xFF, ']'}; json _; CHECK_THROWS_WITH_AS(_ = json::from_bjdata(v1), "[json.exception.parse_error.113] parse error at byte 4: syntax error while parsing BJData size: count in an optimized container must be positive", json::parse_error&); @@ -2535,10 +2644,17 @@ TEST_CASE("BJData") CHECK(json::from_bjdata(vI, true, false).is_discarded()); CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vl), "[json.exception.parse_error.113] parse error at byte 7: syntax error while parsing BJData size: count in an optimized container must be positive", json::parse_error&); - CHECK(json::from_bjdata(vI, true, false).is_discarded()); + CHECK(json::from_bjdata(vl, true, false).is_discarded()); CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vL), "[json.exception.parse_error.113] parse error at byte 11: syntax error while parsing BJData size: count in an optimized container must be positive", json::parse_error&); - CHECK(json::from_bjdata(vI, true, false).is_discarded()); + CHECK(json::from_bjdata(vL, true, false).is_discarded()); + +#if SIZE_MAX == 0xffffffff + 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&); +#else + CHECK_THROWS_WITH_AS(_ = json::from_bjdata(vM), "[json.exception.out_of_range.408] syntax error while parsing BJData size: excessive ndarray size caused overflow", json::out_of_range&); +#endif + CHECK(json::from_bjdata(vM, true, false).is_discarded()); } SECTION("do not accept NTFZ markers in ndarray optimized type (with count)")