Skip to content

Commit

Permalink
Fix ndarray dimension signedness, fix ndarray length overflow (2); ad…
Browse files Browse the repository at this point in the history 
…d 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 <fangqq@gmail.com>
  • Loading branch information
@falbrechtskirchinger @fangq
falbrechtskirchinger and fangq authored Jun 9, 2022
1 parent b6d00d1 commit 48a102c
Show file tree
Hide file tree
Showing 6 changed files with 505 additions and 30 deletions.
18 changes: 17 additions & 1 deletion include/nlohmann/detail/input/binary_reader.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::size_t>(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<std::size_t>(number);
return true;
}
Expand Down Expand Up @@ -2124,6 +2130,12 @@ class binary_reader
{
return false;
}
if (!value_in_range_of<std::size_t>(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<std::size_t>(number);
return true;
}
Expand Down Expand Up @@ -2168,7 +2180,11 @@ class binary_reader
for (auto i : dim)
{
result *= i;
if (JSON_HEDLEY_UNLIKELY(!sax->number_integer(static_cast<number_integer_t>(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<number_unsigned_t>(i))))
{
return false;
}
Expand Down
95 changes: 95 additions & 0 deletions include/nlohmann/detail/meta/type_traits.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -577,5 +577,100 @@ T conditional_static_cast(U value)
return value;
}

template<typename... Types>
using all_integral = conjunction<std::is_integral<Types>...>;

template<typename... Types>
using all_signed = conjunction<std::is_signed<Types>...>;

template<typename... Types>
using all_unsigned = conjunction<std::is_unsigned<Types>...>;

// there's a disjunction trait in another PR; replace when merged
template<typename... Types>
using same_sign = std::integral_constant < bool,
all_signed<Types...>::value || all_unsigned<Types...>::value >;

template<typename OfType, typename T>
using never_out_of_range = std::integral_constant < bool,
(std::is_signed<OfType>::value && (sizeof(T) < sizeof(OfType)))
|| (same_sign<OfType, T>::value && sizeof(OfType) == sizeof(T)) >;

template<typename OfType, typename T,
bool OfTypeSigned = std::is_signed<OfType>::value,
bool TSigned = std::is_signed<T>::value>
struct value_in_range_of_impl2;

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, false, false>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, true, false>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, false, true>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return val >= 0 && static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};


template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, true, true>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) >= static_cast<CommonType>(std::numeric_limits<OfType>::min())
&& static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T,
bool NeverOutOfRange = never_out_of_range<OfType, T>::value,
typename = detail::enable_if_t<all_integral<OfType, T>::value>>
struct value_in_range_of_impl1;

template<typename OfType, typename T>
struct value_in_range_of_impl1<OfType, T, false>
{
static constexpr bool test(T val)
{
return value_in_range_of_impl2<OfType, T>::test(val);
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl1<OfType, T, true>
{
static constexpr bool test(T /*val*/)
{
return true;
}
};

template<typename OfType, typename T>
inline constexpr bool value_in_range_of(T val)
{
return value_in_range_of_impl1<OfType, T>::test(val);
}

} // namespace detail
} // namespace nlohmann
113 changes: 112 additions & 1 deletion single_include/nlohmann/json.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3768,6 +3768,101 @@ T conditional_static_cast(U value)
return value;
}

template<typename... Types>
using all_integral = conjunction<std::is_integral<Types>...>;

template<typename... Types>
using all_signed = conjunction<std::is_signed<Types>...>;

template<typename... Types>
using all_unsigned = conjunction<std::is_unsigned<Types>...>;

// there's a disjunction trait in another PR; replace when merged
template<typename... Types>
using same_sign = std::integral_constant < bool,
all_signed<Types...>::value || all_unsigned<Types...>::value >;

template<typename OfType, typename T>
using never_out_of_range = std::integral_constant < bool,
(std::is_signed<OfType>::value && (sizeof(T) < sizeof(OfType)))
|| (same_sign<OfType, T>::value && sizeof(OfType) == sizeof(T)) >;

template<typename OfType, typename T,
bool OfTypeSigned = std::is_signed<OfType>::value,
bool TSigned = std::is_signed<T>::value>
struct value_in_range_of_impl2;

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, false, false>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, true, false>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, false, true>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return val >= 0 && static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};


template<typename OfType, typename T>
struct value_in_range_of_impl2<OfType, T, true, true>
{
static constexpr bool test(T val)
{
using CommonType = typename std::common_type<OfType, T>::type;
return static_cast<CommonType>(val) >= static_cast<CommonType>(std::numeric_limits<OfType>::min())
&& static_cast<CommonType>(val) <= static_cast<CommonType>(std::numeric_limits<OfType>::max());
}
};

template<typename OfType, typename T,
bool NeverOutOfRange = never_out_of_range<OfType, T>::value,
typename = detail::enable_if_t<all_integral<OfType, T>::value>>
struct value_in_range_of_impl1;

template<typename OfType, typename T>
struct value_in_range_of_impl1<OfType, T, false>
{
static constexpr bool test(T val)
{
return value_in_range_of_impl2<OfType, T>::test(val);
}
};

template<typename OfType, typename T>
struct value_in_range_of_impl1<OfType, T, true>
{
static constexpr bool test(T /*val*/)
{
return true;
}
};

template<typename OfType, typename T>
inline constexpr bool value_in_range_of(T val)
{
return value_in_range_of_impl1<OfType, T>::test(val);
}

} // namespace detail
} // namespace nlohmann

Expand Down Expand Up @@ -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<std::size_t>(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<std::size_t>(number);
return true;
}
Expand Down Expand Up @@ -10714,6 +10815,12 @@ class binary_reader
{
return false;
}
if (!value_in_range_of<std::size_t>(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<std::size_t>(number);
return true;
}
Expand Down Expand Up @@ -10758,7 +10865,11 @@ class binary_reader
for (auto i : dim)
{
result *= i;
if (JSON_HEDLEY_UNLIKELY(!sax->number_integer(static_cast<number_integer_t>(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<number_unsigned_t>(i))))
{
return false;
}
Expand Down
72 changes: 46 additions & 26 deletions tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.")

Expand Down Expand Up @@ -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
$<$<CXX_COMPILER_ID:MSVC>:/EHsc;$<$<CONFIG:Release>:/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
$<$<CXX_COMPILER_ID:MSVC>:/W4 /wd4566 /wd4996>
# https://github.com/nlohmann/json/issues/1114
$<$<CXX_COMPILER_ID:MSVC>:/bigobj> $<$<BOOL:${MINGW}>:-Wa,-mbig-obj>

# https://github.com/nlohmann/json/pull/3229
$<$<CXX_COMPILER_ID:Intel>:-diag-disable=2196>

$<$<NOT:$<CXX_COMPILER_ID:MSVC>>:-Wno-deprecated;-Wno-float-equal>
$<$<CXX_COMPILER_ID:GNU>:-Wno-deprecated-declarations>
$<$<CXX_COMPILER_ID:Intel>:-diag-disable=1786>
)
target_include_directories(test_main PUBLIC
JSON_TEST_KEEP_MACROS)
set(test_main_COMPILE_FEATURES PRIVATE cxx_std_11)
set(test_main_COMPILE_OPTIONS
PUBLIC
$<$<CXX_COMPILER_ID:MSVC>:/EHsc;$<$<CONFIG:Release>:/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
$<$<CXX_COMPILER_ID:MSVC>:/W4 /wd4566 /wd4996>
# https://github.com/nlohmann/json/issues/1114
$<$<CXX_COMPILER_ID:MSVC>:/bigobj> $<$<BOOL:${MINGW}>:-Wa,-mbig-obj>

# https://github.com/nlohmann/json/pull/3229
$<$<CXX_COMPILER_ID:Intel>:-diag-disable=2196>

$<$<NOT:$<CXX_COMPILER_ID:MSVC>>:-Wno-deprecated;-Wno-float-equal>
$<$<CXX_COMPILER_ID:GNU>:-Wno-deprecated-declarations>
$<$<CXX_COMPILER_ID:Intel>:-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
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit 48a102c

Please sign in to comment.