From d3e740c2b3904f66763c03e34a5ec113beeb0df6 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Fri, 27 Sep 2024 11:34:29 +0200 Subject: [PATCH 1/6] Adding more fixes for DoS-able security issues. Adding unit tests for the same. --- CHANGELOG.md | 37 +++ cpp/inc/bond/core/config.h | 2 +- cpp/inc/bond/core/container_interface.h | 6 + cpp/inc/bond/core/detail/typeid_value.h | 127 +++++++--- cpp/inc/bond/core/exception.h | 12 + cpp/inc/bond/core/nullable.h | 22 ++ cpp/inc/bond/core/parser.h | 2 + cpp/inc/bond/core/schema.h | 6 + cpp/inc/bond/core/stl_containers.h | 13 + cpp/inc/bond/core/value.h | 43 +++- cpp/inc/bond/protocol/compact_binary.h | 30 +++ cpp/inc/bond/protocol/fast_binary.h | 28 +++ cpp/inc/bond/protocol/random_protocol.h | 6 + cpp/inc/bond/protocol/simple_binary.h | 65 +++++ cpp/inc/bond/protocol/simple_json_reader.h | 7 + .../bond/protocol/simple_json_reader_impl.h | 11 + cpp/inc/bond/stream/input_buffer.h | 9 +- cpp/test/core/CMakeLists.txt | 6 +- cpp/test/core/container_extensibility.h | 17 ++ cpp/test/core/custom_protocols.h | 5 + cpp/test/core/json_tests.cpp | 122 +++++++++- cpp/test/core/security.bond | 13 + cpp/test/core/security.cpp | 226 ++++++++++++++++++ cpp/test/core/security.h | 6 + cpp/test/core/unit_test_core.bond | 1 - doc/src/bond_cpp.md | 43 ++++ examples/cpp/core/bf/input_file.h | 26 ++ .../container_of_pointers.cpp | 12 + 28 files changed, 848 insertions(+), 55 deletions(-) create mode 100644 cpp/test/core/security.bond create mode 100644 cpp/test/core/security.cpp create mode 100644 cpp/test/core/security.h diff --git a/CHANGELOG.md b/CHANGELOG.md index 028674a815..89f898ea37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,43 @@ tag versions. The Bond compiler (`gbc`) and different versioning scheme, following the Haskell community's [package versioning policy](https://wiki.haskell.org/Package_versioning_policy). +## 13.0.1: 2024-09-30 ## + +### Java ### + +* There were no Java changes in this release. + +### C++ ### + +* `InputBuffer` throws a `StreamException` when trying to skip beyond the + end of the stream. This mitigates a CPU DoS vulnerability. +* Deserialization from JSON payloads will no longer process very deeply + nested structures. Instead, a `bond::CoreException` will be thrown in + order to protect against stack overflows. The depth limit may be changed + by calling the function `bond::SetDeserializeMaxDepth(uint32_t)`. +* **Breaking change**: Protocols must now implement `CanReadArray` method and + Buffers must implement `CanRead` method. These are used to perform checks that + mitigate memory allocation vulnerabilities. +* **Breaking change**: Custom containers must implement `reset_list` and `list_insert`. + Standard implementations are provided. This API is used to incrementally fill + containers of complex types when preallocation may be unsafe. Expected container + size is provided in `reset_list`, where client code can perform sanity checks before + any memory is allocated by Bond. +* `bond::CoreException` is thrown when the payload has a greater declared size + than the backing buffer. + +### C# ### + +* There were no C# changes in this release. + +## 13.0 ## + +This version was allocated but never released. + +## 12.0 ## + +This version was allocated but never released. + ## 11.0.1: 2024-06-26 ## * IDL core version: 3.0 diff --git a/cpp/inc/bond/core/config.h b/cpp/inc/bond/core/config.h index a103845513..fc7fcb32c8 100644 --- a/cpp/inc/bond/core/config.h +++ b/cpp/inc/bond/core/config.h @@ -61,7 +61,7 @@ #define BOND_CONSTEXPR BOOST_CONSTEXPR #define BOND_CONSTEXPR_OR_CONST BOOST_CONSTEXPR_OR_CONST #define BOND_STATIC_CONSTEXPR BOOST_STATIC_CONSTEXPR - +#define BOND_IF_CONSTEXPR BOOST_IF_CONSTEXPR #define BOND_LIB_TYPE_HEADER 1 #define BOND_LIB_TYPE_STATIC 2 diff --git a/cpp/inc/bond/core/container_interface.h b/cpp/inc/bond/core/container_interface.h index 743554ed12..ab9c439ed3 100644 --- a/cpp/inc/bond/core/container_interface.h +++ b/cpp/inc/bond/core/container_interface.h @@ -90,6 +90,12 @@ uint32_t container_size(const T& container); template void resize_list(T& list, uint32_t size); +template +void reset_list(T& list, uint32_t size_hint); + +template +void insert_list(T& list, const E& item); + template void modify_element(T& list, E& element, F deserialize); diff --git a/cpp/inc/bond/core/detail/typeid_value.h b/cpp/inc/bond/core/detail/typeid_value.h index 76dae94f69..b50a68333a 100644 --- a/cpp/inc/bond/core/detail/typeid_value.h +++ b/cpp/inc/bond/core/detail/typeid_value.h @@ -13,6 +13,47 @@ namespace bond { +namespace detail +{ +// The following control which containers will be preallocated when deserializing +// and which require incrementally adding items so that a large size in a payload +// doesn't cause a large memory allocation. +template +struct is_deserialize_direct + : std::false_type {}; + +template +struct is_deserialize_direct::value + && is_element_matching::value>::type> + : std::true_type {}; + +template +struct is_deserialize_direct::value + && !is_nullable::value + && is_basic_type::type>::value + && !require_modify_element::value + && is_element_matching::value>::type> + : std::true_type {}; + +template +struct is_deserialize_incremental + : std::false_type {}; + +template +struct is_deserialize_incremental, TElement> + : std::false_type {}; + +template +struct is_deserialize_incremental::value + && !is_basic_type::type>::value + && is_element_matching::value + && !require_modify_element::value>::type> + : std::true_type {}; +}; + template typename boost::enable_if >::type inline DeserializeMapElements(X& var, const Key& key, const T& element, uint32_t size); @@ -25,14 +66,17 @@ template inline void DeserializeMapElements(const Transform& transform, const Key& key, const T& element, uint32_t size); template -typename boost::enable_if_c::value - && is_element_matching::value>::type +typename boost::enable_if_c::value>::type inline DeserializeElements(X& var, const T& element, uint32_t size); template -typename boost::enable_if >::type +typename boost::enable_if_c::value>::type inline DeserializeElements(nullable& var, const T& element, uint32_t size); +template +typename boost::enable_if_c::value>::type +inline DeserializeElements(X& var, const T& element, uint32_t size); + template inline void DeserializeElements(blob& var, const value& element, uint32_t size); @@ -42,11 +86,11 @@ typename boost::enable_if_c::value inline DeserializeElements(X& var, const T& element, uint32_t size); template -typename boost::disable_if >::type +typename boost::enable_if_c::value >::type inline DeserializeElements(X&, const T& element, uint32_t size); template -inline void DeserializeElements(const Transform& transform, const T& element, uint32_t size); +void inline DeserializeElements(const Transform& transform, const T& element, uint32_t size); namespace detail { @@ -157,6 +201,23 @@ inline bool BasicTypeField(uint16_t id, const Metadata& metadata, BondDataType t } +template +inline void CheckInputData(Reader& input, uint32_t size) +{ + if (!input.CanReadArray(size)) + { + OutOfBoundObjectSizeException(); + } +} + +template +inline void DeserializeElementsChecked(T& var, Reader& input, uint32_t size) +{ + CheckInputData(input, size); + return DeserializeElements(var, value(input, false), size); +} + + template inline void BasicTypeContainer(T& var, BondDataType type, Reader& input, uint32_t size) { @@ -165,43 +226,43 @@ inline void BasicTypeContainer(T& var, BondDataType type, Reader& input, uint32_ switch (type) { case bond::BT_BOOL: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT8: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT16: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT32: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT64: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_FLOAT: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_DOUBLE: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_STRING: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_WSTRING: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT8: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT16: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT32: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT64: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(false); @@ -232,7 +293,7 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t { if (type == get_type_id::type>::value) { - DeserializeElements(var, value::type, Reader&>(input, false), size); + DeserializeElementsChecked::type, Reader>(var, input, size); } else { @@ -250,7 +311,7 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_BOOL: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); @@ -268,7 +329,7 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_STRING: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked < Protocols, T, std::string, Reader > (var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); @@ -286,7 +347,7 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_WSTRING: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); @@ -304,10 +365,10 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_FLOAT: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_DOUBLE: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); @@ -325,16 +386,16 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_UINT8: - return DeserializeElements(var, value(input, false), size); - + return DeserializeElementsChecked(var, input, size); + case bond::BT_UINT16: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT32: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_UINT64: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); @@ -352,16 +413,16 @@ inline MatchingTypeContainer(T& var, BondDataType type, Reader& input, uint32_t switch (type) { case bond::BT_INT8: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT16: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT32: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); case bond::BT_INT64: - return DeserializeElements(var, value(input, false), size); + return DeserializeElementsChecked(var, input, size); default: BOOST_ASSERT(!IsMatching::type>(type)); diff --git a/cpp/inc/bond/core/exception.h b/cpp/inc/bond/core/exception.h index 4387c084cf..50465c9dde 100644 --- a/cpp/inc/bond/core/exception.h +++ b/cpp/inc/bond/core/exception.h @@ -79,6 +79,18 @@ struct CoreException "Max recursion depth exceeded"); } +[[noreturn]] inline void OutOfBoundObjectSizeException() +{ + BOND_THROW(CoreException, + "Payload had an element size larger than the input buffer"); +} + +[[noreturn]] inline void OutOfBoundStringSizeException() +{ + BOND_THROW(CoreException, + "Payload-specified string length exceeds the input buffer size"); +} + namespace detail { template diff --git a/cpp/inc/bond/core/nullable.h b/cpp/inc/bond/core/nullable.h index c6f6af7ba6..34ab58ee68 100644 --- a/cpp/inc/bond/core/nullable.h +++ b/cpp/inc/bond/core/nullable.h @@ -562,6 +562,19 @@ void resize_list(nullable& value, uint32_t size) } } +// reset_list +template +void reset_list(nullable& value, uint32_t /*size_hint*/) +{ + value.reset(); +} + +// insert_list +template +void insert_list(nullable& value, const E& item) +{ + return value.set(item); +} template struct element_type > @@ -633,5 +646,14 @@ template struct is_list_container > : std::true_type {}; +namespace detail +{ +template struct +is_nullable + : std::false_type {}; +template struct +is_nullable > + : std::true_type {}; +}; } // namespace bond diff --git a/cpp/inc/bond/core/parser.h b/cpp/inc/bond/core/parser.h index 90b661b337..c51cd7479b 100644 --- a/cpp/inc/bond/core/parser.h +++ b/cpp/inc/bond/core/parser.h @@ -448,6 +448,8 @@ class DOMParser template bool Apply(const Transform& transform, const Schema& schema) { + detail::RecursionGuard guard; + if (!_base) _input.Parse(); return this->Read(schema, transform); } diff --git a/cpp/inc/bond/core/schema.h b/cpp/inc/bond/core/schema.h index 77566616a4..7878cc3667 100644 --- a/cpp/inc/bond/core/schema.h +++ b/cpp/inc/bond/core/schema.h @@ -165,6 +165,12 @@ namespace detail struct SchemaReader { using Parser = StaticParser; + + template + bool CanReadArray(uint32_t /*num_elems*/) const + { + return true; + } }; diff --git a/cpp/inc/bond/core/stl_containers.h b/cpp/inc/bond/core/stl_containers.h index ba0a089fa3..4d73382f0a 100644 --- a/cpp/inc/bond/core/stl_containers.h +++ b/cpp/inc/bond/core/stl_containers.h @@ -161,6 +161,13 @@ make_element(T& container) return typename element_type::type(container.get_allocator()); } +template +inline +void insert_list(T& list, const E& item) +{ + list.push_back(item); +} + // resize_list template @@ -181,6 +188,12 @@ resize_list(T& list, uint32_t size) list.resize(size, make_element(list)); } +template +inline +void reset_list(T& list, uint32_t/* size_hint*/) +{ + list.clear(); +} // modify_element template diff --git a/cpp/inc/bond/core/value.h b/cpp/inc/bond/core/value.h index 52260c388c..6e9ee09440 100644 --- a/cpp/inc/bond/core/value.h +++ b/cpp/inc/bond/core/value.h @@ -227,7 +227,7 @@ class value_common { Skip(); } - + protected: Reader _input; @@ -675,7 +675,6 @@ inline DeserializeElement(X& var, const I& item, const T& element) modify_element(var, item, DeserializeImpl(element)); } - template typename boost::disable_if >::type inline DeserializeElement(X&, I& item, const T& element) @@ -684,12 +683,12 @@ inline DeserializeElement(X&, I& item, const T& element) } -// Read elements of a list template -typename boost::enable_if_c::value - && is_element_matching::value>::type +typename boost::enable_if_c::value>::type inline DeserializeElements(X& var, const T& element, uint32_t size) { + // In lists of basic types we can easily verify buffer size (and we have done so + // by the time execution gets here), so it is safe to allocate the entire array. resize_list(var, size); for (enumerator items(var); items.more();) @@ -698,9 +697,11 @@ inline DeserializeElements(X& var, const T& element, uint32_t size) template -typename boost::enable_if >::type +typename boost::enable_if_c::value>::type inline DeserializeElements(nullable& var, const T& element, uint32_t size) { + // No need to guard against memory allocation attack here. Since X is nullable, + // at most 1 element will be allocated. resize_list(var, size); for (enumerator > items(var); items.more(); --size) @@ -712,6 +713,28 @@ inline DeserializeElements(nullable& var, const T& element, uint32_t size) detail::SkipElements(element, size); } +template +inline T& get_ref(T& t) noexcept +{ + return t; +} + +template +typename boost::enable_if_c::value>::type +inline DeserializeElements(X& var, const T& element, uint32_t size) +{ + reset_list(var, size); + + // Containers of structs cannot be easily checked. We resort to incrementally + // growing the array. + while (size--) + { + auto e(make_element(var)); + DeserializeElement(var, get_ref(e), element); + insert_list(var, e); + } +} + template inline void DeserializeElements(blob& var, const value& element, uint32_t size) @@ -738,7 +761,7 @@ inline DeserializeElements(X& var, const T& element, uint32_t size) template -typename boost::disable_if >::type +typename boost::enable_if_c::value >::type inline DeserializeElements(X&, const T& element, uint32_t size) { detail::SkipElements(element, size); @@ -806,6 +829,8 @@ inline DeserializeContainer(X& var, const T& element, Reader& input) case bond::BT_LIST: case bond::BT_STRUCT: { + // Buffer check is not needed here since we do not preallocate here. Elements are deserialized + // into a growing array unless the array is of a basic type. if (type == GetTypeId(element)) { DeserializeElements(var, element, size); @@ -818,6 +843,7 @@ inline DeserializeContainer(X& var, const T& element, Reader& input) } default: { + // Buffer checks are performed inside as needed. detail::BasicTypeContainer(var, type, input, size); break; } @@ -840,6 +866,7 @@ inline DeserializeContainer(X& var, const T& element, Reader& input) if (type == GetTypeId(element)) { + // Buffer check is not needed here. We use a growing array for lists of nonbasic types. DeserializeElements(var, element, size); } else @@ -882,6 +909,7 @@ inline DeserializeContainer(X& var, const T& element, Reader& input) } default: { + // Buffer checks performed inside as needed. detail::MatchingTypeContainer(var, type, input, size); break; } @@ -1082,7 +1110,6 @@ inline DeserializeMap(X& var, BondDataType keyType, const T& element, Reader& in input.ReadContainerEnd(); } - } // namespace bond diff --git a/cpp/inc/bond/protocol/compact_binary.h b/cpp/inc/bond/protocol/compact_binary.h index ad8de71d90..87f90b40f7 100644 --- a/cpp/inc/bond/protocol/compact_binary.h +++ b/cpp/inc/bond/protocol/compact_binary.h @@ -373,6 +373,14 @@ class CompactBinaryReader uint32_t length = 0; Read(length); + + constexpr uint8_t charSize = static_cast(sizeof(typename detail::string_char_int_type::type)); + uint32_t numStringBytes = detail::checked_multiply(length, charSize); + if (!_input.CanRead(numStringBytes)) + { + OutOfBoundStringSizeException(); + } + detail::ReadStringData(_input, value, length); } @@ -383,6 +391,28 @@ class CompactBinaryReader _input.Read(value, size); } + // Does the reader have enough input buffer left to read an array of T? + template + bool CanReadArray(uint32_t num_elems) + { + // Non-float types have variable length encoding going down to 1 Byte. + // Strings need 1 Byte per charcter. Wide strings handled below. + return _input.CanRead(num_elems); + } + + template + typename boost::enable_if::value, bool>::type + CanReadArray(uint32_t num_elems) + { + // We will need to read num_elems instances of T. This will not overflow because + // num_elems < 2^32 and we call this only if std::is_floating_point. + uint64_t num_bytes = static_cast(num_elems) * sizeof(T); + + // Check the upper half to ensure we don't try to read more than 4 GB, the + // Reader wouldn't be able to handle that. Then ask the Reader if it has enough + // data left for our vector. + return (num_bytes >> 32 == 0) && _input.CanRead(static_cast(num_bytes & 0xffffffff)); + } template void Skip() diff --git a/cpp/inc/bond/protocol/fast_binary.h b/cpp/inc/bond/protocol/fast_binary.h index b3f5e061cf..d550553375 100644 --- a/cpp/inc/bond/protocol/fast_binary.h +++ b/cpp/inc/bond/protocol/fast_binary.h @@ -181,6 +181,12 @@ class FastBinaryReader uint32_t length = 0; ReadVariableUnsigned(_input, length); + + constexpr uint8_t charSize = static_cast(sizeof(typename detail::string_char_int_type::type)); + uint32_t numStringBytes = detail::checked_multiply(length, charSize); + if (!_input.CanRead(numStringBytes)) + OutOfBoundStringSizeException(); + detail::ReadStringData(_input, value, length); } @@ -191,6 +197,28 @@ class FastBinaryReader _input.Read(value, size); } + // Does the reader have enough input buffer left to read an array of T? + template + bool CanReadArray(uint32_t num_elems) + { + // We will need to read num_elems instances of T. This will not overflow because + // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. + uint64_t num_bytes = static_cast(num_elems) * sizeof(T); + return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); + } + + template<> + bool CanReadArray(uint32_t num_elems) + { + return _input.CanRead(num_elems); + } + + template<> + bool CanReadArray(uint32_t num_elems) + { + return _input.CanRead(num_elems); + } + void ReadStructBegin() {} diff --git a/cpp/inc/bond/protocol/random_protocol.h b/cpp/inc/bond/protocol/random_protocol.h index 452c83f01d..351dc92e67 100644 --- a/cpp/inc/bond/protocol/random_protocol.h +++ b/cpp/inc/bond/protocol/random_protocol.h @@ -193,6 +193,12 @@ class RandomProtocolReader value.assign(buffer, size); } + template + bool CanReadArray(uint32_t /*num_elems*/) const + { + return true; + } + template void Skip() {} diff --git a/cpp/inc/bond/protocol/simple_binary.h b/cpp/inc/bond/protocol/simple_binary.h index 059b1e171d..8b8cbefe60 100644 --- a/cpp/inc/bond/protocol/simple_binary.h +++ b/cpp/inc/bond/protocol/simple_binary.h @@ -165,6 +165,14 @@ class SimpleBinaryReader uint32_t length = 0; ReadSize(length); + + constexpr uint8_t charSize = static_cast(sizeof(typename detail::string_char_int_type::type)); + uint32_t numStringBytes = detail::checked_multiply(length, charSize); + if (!_input.CanRead(numStringBytes)) + { + OutOfBoundStringSizeException(); + } + detail::ReadStringData(_input, var, length); } @@ -175,6 +183,63 @@ class SimpleBinaryReader _input.Read(var, size); } + // Does the reader have enough input buffer left to read an array of T? + template + bool CanReadArray(uint32_t num_elems) + { + // We will need to read num_elems instances of T. This will not overflow because + // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. + uint64_t num_bytes = static_cast(num_elems) * sizeof(T); + + // Check if num_bytes is 32-bit as the Reader cannot grab more than that + return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); + } + + template<> + bool CanReadArray(uint32_t num_elems) + { + // booleans are encoded as 1 Byte + return _input.CanRead(num_elems); + } + + template<> + bool CanReadArray(uint32_t num_elems) + { + // This is a compile-time compare. In C++17 this problem does not exist. +#ifdef _MSC_VER + #pragma warning(push) + #pragma warning(disable:4127) +#endif + BOND_IF_CONSTEXPR(version == v1) + { + // In v1, strings encode their length as uin32 in 4 Bytes, so we multiply num_elems. + return _input.CanRead(detail::checked_multiply(num_elems, 4)); + } + else + { + // In v2, strings use variable-length encoded integers to specify length, 1 Byte each + // ix their minumum length each. + return _input.CanRead(num_elems); + } +#ifdef _MSC_VER + #pragma warning(pop) +#endif + } + + template<> + bool CanReadArray(uint32_t num_elems) + { + // This is a compile-time compare. In C++17 this problem does not exist. + #pragma warning(push) + #pragma warning(disable:4127) + + BOND_IF_CONSTEXPR (version == v1) + return _input.CanRead(detail::checked_multiply(num_elems, 4)); + else + return _input.CanRead(num_elems); + + #pragma warning(pop) + } // Skip for basic types template diff --git a/cpp/inc/bond/protocol/simple_json_reader.h b/cpp/inc/bond/protocol/simple_json_reader.h index 82e91209e9..c7dcae7ee7 100644 --- a/cpp/inc/bond/protocol/simple_json_reader.h +++ b/cpp/inc/bond/protocol/simple_json_reader.h @@ -95,6 +95,13 @@ class SimpleJsonReader detail::Read(*GetValue(), var); } + // Does the reader have enough input buffer left to read an array of T? + template + bool CanReadArray(uint32_t /*num_elems*/) const + { + return true; + } + template void ReadContainerBegin(uint32_t&, T&) { diff --git a/cpp/inc/bond/protocol/simple_json_reader_impl.h b/cpp/inc/bond/protocol/simple_json_reader_impl.h index bd61336224..a6c0e67de3 100644 --- a/cpp/inc/bond/protocol/simple_json_reader_impl.h +++ b/cpp/inc/bond/protocol/simple_json_reader_impl.h @@ -6,6 +6,7 @@ #include #include "simple_json_reader.h" +#include "detail/recursionguard.h" namespace bond { @@ -49,6 +50,8 @@ SimpleJsonReader::FindField(uint16_t id, const Metadata& metadata, Bond template inline void DeserializeContainer(std::vector& var, const T& /*element*/, SimpleJsonReader& reader) { + bond::detail::RecursionGuard guard; + rapidjson::Value::ConstValueIterator it = reader.ArrayBegin(); resize_list(var, reader.ArraySize()); @@ -63,6 +66,8 @@ inline void DeserializeContainer(std::vector& var, const T& /*element*/ template inline void DeserializeContainer(blob& var, const T& /*element*/, SimpleJsonReader& reader) { + bond::detail::RecursionGuard guard; + if (uint32_t size = reader.ArraySize()) { boost::shared_ptr buffer = boost::make_shared_noinit(size); @@ -84,6 +89,8 @@ template inline typename boost::enable_if >::type DeserializeContainer(X& var, const T& element, SimpleJsonReader& reader) { + bond::detail::RecursionGuard guard; + detail::JsonTypeMatching type(get_type_id::type>::value, GetTypeId(element), std::is_enum::type>::value); @@ -116,6 +123,8 @@ template inline typename boost::enable_if >::type DeserializeContainer(X& var, const T& element, SimpleJsonReader& reader) { + bond::detail::RecursionGuard guard; + detail::JsonTypeMatching type(get_type_id::type>::value, GetTypeId(element), std::is_enum::type>::value); @@ -139,6 +148,8 @@ template inline typename boost::enable_if >::type DeserializeMap(X& var, BondDataType keyType, const T& element, SimpleJsonReader& reader) { + bond::detail::RecursionGuard guard; + detail::JsonTypeMatching key_type( get_type_id::type::first_type>::value, keyType, diff --git a/cpp/inc/bond/stream/input_buffer.h b/cpp/inc/bond/stream/input_buffer.h index 8dc6a26e85..e0ce7d3805 100644 --- a/cpp/inc/bond/stream/input_buffer.h +++ b/cpp/inc/bond/stream/input_buffer.h @@ -194,7 +194,7 @@ class InputBuffer { if (size > _blob.length() - _pointer) { - return; + EofException(size); } _pointer += size; @@ -206,6 +206,11 @@ class InputBuffer return _pointer == _blob.length(); } + /// @brief Check if the stream can read at least @size bytes before encountering end of stream. + bool CanRead(uint32_t size) const + { + return size <= _blob.length() - _pointer; + } template void ReadVariableUnsigned(T& value) @@ -226,7 +231,7 @@ class InputBuffer [[noreturn]] void EofException(uint32_t size) const { BOND_THROW(StreamException, - "Read out of bounds: " << size << " bytes requested, offset: " + "Read or skip out of bounds: " << size << " bytes requested, offset: " << _pointer << ", length: " << _blob.length()); } diff --git a/cpp/test/core/CMakeLists.txt b/cpp/test/core/CMakeLists.txt index 00cb22d921..136af3a4e9 100644 --- a/cpp/test/core/CMakeLists.txt +++ b/cpp/test/core/CMakeLists.txt @@ -76,13 +76,14 @@ add_bond_codegen (TARGET unit_test_codegen1 --header=\\\"container_extensibility.h\\\") add_bond_codegen (TARGET unit_test_codegen2 - unit_test_core.bond apply_test.bond + cmdargs.bond import_test1.bond scope_test1.bond scope_test2.bond + security.bond + unit_test_core.bond validation.bond - cmdargs.bond OPTIONS --import-dir=imports --header=\\\"custom_protocols.h\\\") @@ -147,6 +148,7 @@ add_unit_test (numeric_conversions.cpp) add_unit_test (pass_through.cpp) add_unit_test (protocol_test.cpp) add_unit_test (required_fields_tests.cpp) +add_unit_test (security.cpp) add_unit_test (serialization_test.cpp) add_unit_test (set_tests.cpp) add_unit_test (skip_id_tests.cpp) diff --git a/cpp/test/core/container_extensibility.h b/cpp/test/core/container_extensibility.h index ddf7a9820a..2a4525c0c2 100644 --- a/cpp/test/core/container_extensibility.h +++ b/cpp/test/core/container_extensibility.h @@ -130,6 +130,23 @@ void resize_list(SimpleList& list, uint32_t size) list.size = size; } +// reset_list +template +void reset_list(SimpleList& list, uint32_t size) +{ + BOOST_ASSERT(size <= c_max_list_size); + list.size = 0; +} + +// insert_list +template +inline +void insert_list(SimpleList& list, const E& item) +{ + BOOST_ASSERT(list.size < c_max_list_size); + list.items[list.size] = item; + list.size++; +} namespace bond { diff --git a/cpp/test/core/custom_protocols.h b/cpp/test/core/custom_protocols.h index e39f0f17c5..170fcd4718 100644 --- a/cpp/test/core/custom_protocols.h +++ b/cpp/test/core/custom_protocols.h @@ -113,6 +113,11 @@ namespace unit_test return _buffer.IsEof(); } + bool CanRead(uint32_t size) const + { + return _buffer.CanRead(size); + } + private: friend bond::blob GetCurrentBuffer(const CustomInputBuffer& input) { diff --git a/cpp/test/core/json_tests.cpp b/cpp/test/core/json_tests.cpp index 820eda8c4d..20c9b4e9f5 100644 --- a/cpp/test/core/json_tests.cpp +++ b/cpp/test/core/json_tests.cpp @@ -225,26 +225,128 @@ TEST_CASE_BEGIN(ReaderOverCStr) } TEST_CASE_END -TEST_CASE_BEGIN(DeepNesting) +TEST_CASE_BEGIN(SimpleType_DeepNestedPayloadOfArray) { const size_t nestingDepth = 10000; std::string listOpens(nestingDepth, '['); std::string listCloses(nestingDepth, ']'); - std::string deeplyNestedList = boost::str( + std::string deeplyNestedJson = boost::str( boost::format("{\"deeplyNestedList\": %strue%s}") % listOpens % listCloses); - bond::SimpleJsonReader json_reader(deeplyNestedList.c_str()); + bond::SimpleJsonReader json_reader(deeplyNestedJson.c_str()); - // The type here doesn't really matter. We need something with no - // required fields, as we're really just testing that we can parse a - // deeply nested JSON array without crashing. + // The type here doesn't really matter. We need something with no struct + // or container fields, as we're really just testing that RapidJSON can + // parse a deeply nested JSON array without throwing. SimpleStruct to; bond::Deserialize(json_reader, to); } TEST_CASE_END +TEST_CASE_BEGIN(SimpleType_DeepNestedPayloadOfStruct) +{ + const size_t nestingDepth = 10000; + + std::string deeplyNestedJson; + + for (int i = 0; i < nestingDepth; i++) + { + deeplyNestedJson += "{\"a\":"; + } + + deeplyNestedJson += "\"some-inner-most-value\""; + + for (int i = 0; i < nestingDepth; i++) + { + deeplyNestedJson += "}"; + } + + bond::SimpleJsonReader json_reader(deeplyNestedJson.c_str()); + + // The type here doesn't really matter. We need something with no struct + // or container fields, as we're really just testing that RapidJSON can + // parse a deeply nested JSON object without throwing. + SimpleStruct to; + bond::Deserialize(json_reader, to); +} +TEST_CASE_END + +TEST_CASE_BEGIN(RecursiveType_ThrowsOnTooManyNestings) +{ + const size_t nestingLevel = 64; + + std::string deeplyNestedJson; + + for (int i = 0; i < nestingLevel; i++) + { + deeplyNestedJson += "{\"Child\":["; + } + + for (int i = 0; i < nestingLevel; i++) + { + deeplyNestedJson += "]}"; + } + + StructWithRecursiveReference to; + bond::SimpleJsonReader json_reader_from_string(deeplyNestedJson.c_str()); + + // Validate deserialization throws + UT_AssertThrows((bond::Deserialize(json_reader_from_string, to)), bond::CoreException); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SetDeserializeMaxDepth_ImpactsValidWorkloads) +{ + NestedStruct3 from; + InitRandom(from); + + // Default max depth + { + bond::OutputBuffer output; + bond::SimpleJsonWriter json_writer(output); + + Serialize(from, json_writer); + + bond::SimpleJsonReader json_reader(output.GetBuffer()); + NestedStruct3 to; + Deserialize(json_reader, to); + UT_Equal(from, to); + } + + // Lower the depth to the point where a valid workload fails + { + bond::OutputBuffer output; + bond::SimpleJsonWriter json_writer(output); + + Serialize(from, json_writer); + + bond::SetDeserializeMaxDepth(1); + + bond::SimpleJsonReader json_reader(output.GetBuffer()); + NestedStruct3 to; + UT_AssertThrows((bond::Deserialize(json_reader, to)), bond::CoreException); + } + + // Put the depth back and validate the workload works again + { + bond::OutputBuffer output; + bond::SimpleJsonWriter json_writer(output); + + Serialize(from, json_writer); + + bond::SetDeserializeMaxDepth(64); + + bond::SimpleJsonReader json_reader(output.GetBuffer()); + NestedStruct3 to; + Deserialize(json_reader, to); + UT_Equal(from, to); + } +} +TEST_CASE_END + + void JSONTest::Initialize() { UnitTestSuite suite("Simple JSON test"); @@ -256,8 +358,12 @@ void JSONTest::Initialize() bond::SimpleJsonWriter >(suite); ); - AddTestCase(suite, "Deeply nested JSON struct"); - AddTestCase(suite, "SimpleJsonReader specialization"); + AddTestCase(suite, "SimpleJsonReader specialization"); + AddTestCase(suite, "Simple struct can be parsed from payload with deeply nested JSON array"); + AddTestCase(suite, "Simple struct can be parsed from payload with deeply nested JSON object"); + AddTestCase(suite, "Too many nestings in recursive type results in exception"); + AddTestCase(suite, "Test that SetDeserializeMaxDepth has an effect"); + } bool init_unit_test() diff --git a/cpp/test/core/security.bond b/cpp/test/core/security.bond new file mode 100644 index 0000000000..130be54080 --- /dev/null +++ b/cpp/test/core/security.bond @@ -0,0 +1,13 @@ +namespace security + +struct Base +{ + 0: T x; +} + +struct Struct : Base +{ + 0: T3 n; + 1: T1 y; + 2: vector items; +} diff --git a/cpp/test/core/security.cpp b/cpp/test/core/security.cpp new file mode 100644 index 0000000000..451d078411 --- /dev/null +++ b/cpp/test/core/security.cpp @@ -0,0 +1,226 @@ +#include "precompiled.h" +#include "security.h" +#include + +#include "security_reflection.h" +#include "security_types.h" + +using namespace security; +typedef uint32_t uint; + +TEST_CASE_BEGIN(SecurityBadAlloc_VectorPrimitive) +{ + const unsigned char pMaliciousPayload[] = { + 0xA5, 0xA5, 0x4B, 0x0D, 0x4B, 0x0D, 0x00, 0x41, + 0x4B, 0xE7, 0xAA, 0xBB, 0xCC, 0xDD, 0xEE }; + + // create our object based on our fuzzed bond types file + Struct > obj; + + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::CoreException); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_VectorStruct) +{ + // The payload specifies a lot of items in Struct::items but encodes only one. + const unsigned char pMaliciousPayload[] = { + 0x0A, 0x00, 0x01, 0x05, 0x03, 0x25, 0x04, 0x4B, + 0x0A, 0xFF, 0xFF, 0xFF, 0x0F, 0x05, 0x7F, 0x00, + 0x00 }; + + Struct, uint> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + // We will reach EOF while reading the second item in Struct::items, expect a throw. + UT_AssertThrows(bond::Deserialize(reader, obj), bond::StreamException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.items.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_MapPrimitive) +{ + const unsigned char pMaliciousPayload[] = { + 0x0d, 0x10, 0x10, 0x7f, 0x02, 0xd0, 0x0f, 0x00 }; + + Base> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::StreamException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.x.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_SetPrimitive) +{ + const unsigned char pMaliciousPayload[] = { + 0x0c, 0x10, 0x7f, 0x02, 0x00 }; + + Base> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::CoreException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.x.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_ListPrimitive) +{ + const unsigned char pMaliciousPayload[] = { + 0x0b, 0x07, 0x7f, 0x00, 0x00, 0x80, 0x3f, 0x00 }; + + Base> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::CoreException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.x.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_ListStruct) +{ + const unsigned char pMaliciousPayload[] = { + 0x0b, 0x0a, 0x7f, 0x10, 0x02, 0x01, 0x10, 0x01, + 0x30, 0x04, 0x00, 0x00 }; + + Base>> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::CoreException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.x.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityBadAlloc_MapStruct) +{ + const unsigned char pMaliciousPayload[] = { + 0x0d, 0x10, 0x0a, 0x7f, 0x02, 0x07, 0x00, 0x00, + 0x80, 0x3f, 0x01, 0x07, 0x00, 0x00, 0x40, 0x40, + 0x27, 0x00, 0x00, 0x00, 0x40, 0x00, 0x00 }; + + Base>> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::StreamException); + + // We must not allocate more fields than are encoded + 1. + UT_AssertIsTrue(obj.x.size() <= 2); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityNullable_MustSkip) +{ + // We put 2 items into a nullable which may have only 0 or 1. The test + // makes sure that the remaining items are skipped and do not corrupt + // the rest of deserialization. + const unsigned char pMaliciousPayload[] = { + 0x01, 0x0b, 0x10, 0x02, 0x08, 0x0a, 0x30, 0x02, + 0x00 }; + + Struct> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + bond::Deserialize(reader, obj); + + UT_AssertIsTrue(obj.x == 0); + UT_AssertIsTrue(obj.y == 1); + UT_AssertIsTrue(obj.n.hasvalue() && (obj.n.value() == 4 || obj.n.value() == 5)); + UT_AssertIsTrue(obj.items.size() == 0); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityNullable_InsufficientBuffer) +{ + // Payload spcifies a nullable with 127 items, while there are only + // 4 Bytes left in the buffer. + const unsigned char pMaliciousPayload[] = { + 0x01, 0x0b, 0x10, 0x7f, 0x08, 0x30, 0x02, + 0x00 }; + + Struct> obj; + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::CoreException); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityCpuDos_FastBinary) +{ + const unsigned char pMaliciousPayload[] = { + 0x0D, 0x00, 0x96, 0x08, 0x0E, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF }; + + // create our object based on our fuzzed bond types file + Struct > obj; + + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::FastBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::StreamException); +} +TEST_CASE_END + +TEST_CASE_BEGIN(SecurityCpuDos_CompactBinary) +{ + const unsigned char pMaliciousPayload[] = { + 0x01, 0x2D, 0x0E, 0x08, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xAB, + 0x09}; + + // create our object based on our fuzzed bond types file + Struct > obj; + + bond::InputBuffer input_buffer(static_cast(pMaliciousPayload), sizeof(pMaliciousPayload)); + bond::CompactBinaryReader reader(input_buffer); + + UT_AssertThrows(bond::Deserialize(reader, obj), bond::StreamException); +} +TEST_CASE_END + +void SecurityTest::Initialize() +{ + UnitTestSuite suite("Security tests"); + AddTestCase(suite, "CpuDos_FastBinary"); + AddTestCase(suite, "CpuDos_CompactBinary"); + + AddTestCase(suite, "BadAlloc_VectorPrimitive"); + AddTestCase(suite, "BadAlloc_VectorStruct"); + AddTestCase(suite, "BadAlloc_MapPrimitive"); + AddTestCase(suite, "BadAlloc_MapStruct"); + AddTestCase(suite, "BadAlloc_SetPrimitive"); + AddTestCase(suite, "BadAlloc_ListPrimitive"); + AddTestCase(suite, "BadAlloc_ListStruct"); + + AddTestCase(suite, "Nullable_MustSkip"); + AddTestCase(suite, "Nullable_InsufficientBuffer"); +} + + +bool init_unit_test() +{ + SecurityTest::Initialize(); + return true; +} diff --git a/cpp/test/core/security.h b/cpp/test/core/security.h new file mode 100644 index 0000000000..2b0ad9420a --- /dev/null +++ b/cpp/test/core/security.h @@ -0,0 +1,6 @@ + +class SecurityTest +{ +public: + static void Initialize(); +}; diff --git a/cpp/test/core/unit_test_core.bond b/cpp/test/core/unit_test_core.bond index b50843eb5a..e0e10362cb 100644 --- a/cpp/test/core/unit_test_core.bond +++ b/cpp/test/core/unit_test_core.bond @@ -29,7 +29,6 @@ struct SimpleOptionalsView view_of SimpleOptionals b; }; - struct SimpleStruct { 0: required_optional bool m_bool; diff --git a/doc/src/bond_cpp.md b/doc/src/bond_cpp.md index 74a256cd1e..cbf2374ff1 100644 --- a/doc/src/bond_cpp.md +++ b/doc/src/bond_cpp.md @@ -1487,6 +1487,14 @@ uint32_t container_size(const T& container); template void resize_list(T& list, uint32_t size); +// Added in Bond 13. See note below. +template +void reset_list(T& list, uint32_t size_hint); + +// Added in Bond 13. See note below. +template +void insert_list(T& list, const E& item); + template void clear_set(T& set); @@ -1500,6 +1508,20 @@ template T& mapped_at(M& map, const K& key); ``` +> **NOTE** The functions `reset_list` and `insert_list` need to be implemented +> carefully to guard against DoS attacks through excessive memory allocation. +> The function `reset_list` is called by Bond to prepare a container for inserting +> deserialized elements. The application should examine `size_hint`; if this value +> is unreasonably high, an exception should be thrown to signal error. Otherwise, +> the client code should clear the container but __refrain from allocating__ any memory +> for the data. This is because Bond is unable to verify that the payload actually +> contains the declared number of items. Bond will then iteratively deserialize and +> insert individual items into the container by calling `insert_list`. Custom +> implementations should insert the provided item into the collection. It is a good +> practice to check that the number of items in the container does not exceed +> `size_hint`. + + Note that unlike the traits which need to be specialized in the `bond` namespace, these function can be overloaded in the namespace of the container type. @@ -1651,6 +1673,20 @@ typename aliased_type::type get_aliased_value(const T& value); - `examples/cpp/core/time_alias` +Smart pointers +-------------- +References to values are obtained with `get_ref`: +```cpp +template +inline T& get_ref(T& t); +``` + +To use containers of smart pointers, Bond needs to make sure that an object is +constructed before its reference is taken. In `get_ref` that object is created +if needed. + +- `examples/cpp/core/container_of_pointers` + Custom allocators ================= @@ -1711,6 +1747,13 @@ public: // Read into a memory blob void Read(bond::blob& blob, uint32_t size); + + // Advance buffer pointer by size Bytes without returning the data. If size is + // smaller than the remaining buffer size, throw a StreamException. + void Skip(uint32_t size); + + // Return true iff the next Read of size Bytes succeeds. + bool CanRead(uint32_t size) const; }; ``` diff --git a/examples/cpp/core/bf/input_file.h b/examples/cpp/core/bf/input_file.h index 882f3778d0..2c9b422958 100644 --- a/examples/cpp/core/bf/input_file.h +++ b/examples/cpp/core/bf/input_file.h @@ -24,6 +24,7 @@ class InputFile if (file.good()) { file.exceptions(std::ifstream::failbit | std::ifstream::badbit | std::ifstream::eofbit); + InitLength(); } else { @@ -40,6 +41,7 @@ class InputFile { file.exceptions(std::ifstream::failbit | std::ifstream::badbit | std::ifstream::eofbit); file.seekg(that.file.tellg()); + InitLength(); } else { @@ -71,6 +73,11 @@ class InputFile blob.assign(buffer, 0, size); } + bool CanRead(uint32_t size) const + { + return size <= file_size - file.tellg(); + } + void Skip(uint32_t size) { file.seekg(size, std::ios::cur); @@ -84,6 +91,25 @@ class InputFile private: mutable std::ifstream file; std::string name; + std::streampos file_size; + + void InitLength() + { + const std::streampos orig_pos = file.tellg(); + + file.seekg(0, file.end); + if (file.fail()) + { + // The file is not seekable. Set size to maximum int to remove restrictions. + file_size = std::numeric_limits::max(); + file.clear(); + } + else + { + file_size = file.tellg(); + file.seekg(orig_pos, file.beg); + } + } }; diff --git a/examples/cpp/core/container_of_pointers/container_of_pointers.cpp b/examples/cpp/core/container_of_pointers/container_of_pointers.cpp index 9511a6fbdf..2839b860d5 100644 --- a/examples/cpp/core/container_of_pointers/container_of_pointers.cpp +++ b/examples/cpp/core/container_of_pointers/container_of_pointers.cpp @@ -101,6 +101,18 @@ namespace bond A alloc; typename List::iterator it, end; }; + + // element + template + inline T& get_ref(std::shared_ptr& t) + { + if (!t) + { + t = std::make_shared(); + } + + return *t; + } } #include From 01410ca6705f15c39d961932986a3733cc507fde Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Mon, 30 Sep 2024 15:18:40 +0200 Subject: [PATCH 2/6] Adding versions to changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 89f898ea37..3f02e12ca1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,12 @@ different versioning scheme, following the Haskell community's ## 13.0.1: 2024-09-30 ## +* IDL core version: 3.0 +* C++ version: 13.0.1 +* C# NuGet version: 13.0.1 +* Java version: 13.0.1 +* `gbc` & compiler library: 0.13.0.0 + ### Java ### * There were no Java changes in this release. From d91dfdc804958cb5a56f58ae59577ae1a5e6f8bd Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Tue, 1 Oct 2024 09:02:59 +0200 Subject: [PATCH 3/6] Improving compatibility with gcc and clang. --- cpp/inc/bond/core/detail/typeid_value.h | 2 +- cpp/inc/bond/core/value.h | 3 +- cpp/inc/bond/protocol/fast_binary.h | 34 +++++---- cpp/inc/bond/protocol/simple_binary.h | 75 ++++++++----------- .../bond/protocol/simple_json_reader_impl.h | 2 +- 5 files changed, 54 insertions(+), 62 deletions(-) diff --git a/cpp/inc/bond/core/detail/typeid_value.h b/cpp/inc/bond/core/detail/typeid_value.h index b50a68333a..284711eaf9 100644 --- a/cpp/inc/bond/core/detail/typeid_value.h +++ b/cpp/inc/bond/core/detail/typeid_value.h @@ -204,7 +204,7 @@ inline bool BasicTypeField(uint16_t id, const Metadata& metadata, BondDataType t template inline void CheckInputData(Reader& input, uint32_t size) { - if (!input.CanReadArray(size)) + if (!input.template CanReadArray(size)) { OutOfBoundObjectSizeException(); } diff --git a/cpp/inc/bond/core/value.h b/cpp/inc/bond/core/value.h index 6e9ee09440..bbb537b236 100644 --- a/cpp/inc/bond/core/value.h +++ b/cpp/inc/bond/core/value.h @@ -4,11 +4,10 @@ #pragma once #include - +#include #include "bonded.h" #include "protocol.h" #include "schema.h" -#include "detail/recursionguard.h" #include "detail/typeid_value.h" #include diff --git a/cpp/inc/bond/protocol/fast_binary.h b/cpp/inc/bond/protocol/fast_binary.h index d550553375..e027ab3f68 100644 --- a/cpp/inc/bond/protocol/fast_binary.h +++ b/cpp/inc/bond/protocol/fast_binary.h @@ -201,22 +201,26 @@ class FastBinaryReader template bool CanReadArray(uint32_t num_elems) { - // We will need to read num_elems instances of T. This will not overflow because - // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. - uint64_t num_bytes = static_cast(num_elems) * sizeof(T); - return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); - } +#ifdef _MSC_VER +#pragma warning(push) +#pragma warning(disable:4127) +#endif - template<> - bool CanReadArray(uint32_t num_elems) - { - return _input.CanRead(num_elems); - } + BOND_IF_CONSTEXPR(is_string_type::value) + { + return _input.CanRead(num_elems); + } + else + { + // We will need to read num_elems instances of T. This will not overflow because + // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. + uint64_t num_bytes = static_cast(num_elems) * sizeof(T); + return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); + } - template<> - bool CanReadArray(uint32_t num_elems) - { - return _input.CanRead(num_elems); +#ifdef _MSC_VER +#pragma warning(pop) +#endif } void ReadStructBegin() @@ -607,4 +611,4 @@ class FastBinaryWriter }; -} // namespace bond +}; // namespace bond diff --git a/cpp/inc/bond/protocol/simple_binary.h b/cpp/inc/bond/protocol/simple_binary.h index 8b8cbefe60..3e929ad7e3 100644 --- a/cpp/inc/bond/protocol/simple_binary.h +++ b/cpp/inc/bond/protocol/simple_binary.h @@ -184,61 +184,50 @@ class SimpleBinaryReader } // Does the reader have enough input buffer left to read an array of T? - template + template bool CanReadArray(uint32_t num_elems) { - // We will need to read num_elems instances of T. This will not overflow because - // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. - uint64_t num_bytes = static_cast(num_elems) * sizeof(T); - - // Check if num_bytes is 32-bit as the Reader cannot grab more than that - return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); - } - - template<> - bool CanReadArray(uint32_t num_elems) - { - // booleans are encoded as 1 Byte - return _input.CanRead(num_elems); - } - - template<> - bool CanReadArray(uint32_t num_elems) - { - // This is a compile-time compare. In C++17 this problem does not exist. + // We need to silence MSVC's warning about constant conditional expression. This + // construct could be replaced with explicit template specialization, but that is + // not possible due to gcc's bug (https://open-std.org/JTC1/SC22/WG21/docs/cwg_defects.html#727). #ifdef _MSC_VER - #pragma warning(push) - #pragma warning(disable:4127) +#pragma warning(push) +#pragma warning(disable:4127) #endif - BOND_IF_CONSTEXPR(version == v1) + + BOND_IF_CONSTEXPR(is_string_type::value) { - // In v1, strings encode their length as uin32 in 4 Bytes, so we multiply num_elems. - return _input.CanRead(detail::checked_multiply(num_elems, 4)); + BOND_IF_CONSTEXPR(version == v1) + { + // In v1, strings encode their length as uin32 in 4 Bytes, so we multiply num_elems. + return _input.CanRead(detail::checked_multiply(num_elems, 4)); + } + else + { + // In v2, strings use variable-length encoded integers to specify length, 1 Byte each + // ix their minumum length each. + return _input.CanRead(num_elems); + } } - else + else BOND_IF_CONSTEXPR(boost::is_same::value) { - // In v2, strings use variable-length encoded integers to specify length, 1 Byte each - // ix their minumum length each. + // booleans are encoded as 1 Byte return _input.CanRead(num_elems); } -#ifdef _MSC_VER - #pragma warning(pop) -#endif - } + else + { + // We will need to read num_elems instances of T. This will not overflow because + // num_elems < 2^32 and we call this only for primitive types, so sizeof(T) <= 8. + uint64_t num_bytes = static_cast(num_elems) * sizeof(T); - template<> - bool CanReadArray(uint32_t num_elems) - { - // This is a compile-time compare. In C++17 this problem does not exist. - #pragma warning(push) - #pragma warning(disable:4127) + // Check if num_bytes is 32-bit as the Reader cannot grab more than that + return (num_bytes >> 32 == 0) && _input.CanRead(num_bytes & 0xffffffff); + } - BOND_IF_CONSTEXPR (version == v1) - return _input.CanRead(detail::checked_multiply(num_elems, 4)); - else - return _input.CanRead(num_elems); +#ifdef _MSC_VER +#pragma warning(pop) +#endif - #pragma warning(pop) } // Skip for basic types diff --git a/cpp/inc/bond/protocol/simple_json_reader_impl.h b/cpp/inc/bond/protocol/simple_json_reader_impl.h index a6c0e67de3..e2cd393c4a 100644 --- a/cpp/inc/bond/protocol/simple_json_reader_impl.h +++ b/cpp/inc/bond/protocol/simple_json_reader_impl.h @@ -4,9 +4,9 @@ #pragma once #include +#include #include "simple_json_reader.h" -#include "detail/recursionguard.h" namespace bond { From 359ecf1f6fe1239fea69fa8ab69fea1355fc6d77 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Tue, 1 Oct 2024 13:34:44 +0200 Subject: [PATCH 4/6] Adding missing constexpr if --- cpp/inc/bond/core/config.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/inc/bond/core/config.h b/cpp/inc/bond/core/config.h index fc7fcb32c8..f4e3cb5d22 100644 --- a/cpp/inc/bond/core/config.h +++ b/cpp/inc/bond/core/config.h @@ -61,7 +61,12 @@ #define BOND_CONSTEXPR BOOST_CONSTEXPR #define BOND_CONSTEXPR_OR_CONST BOOST_CONSTEXPR_OR_CONST #define BOND_STATIC_CONSTEXPR BOOST_STATIC_CONSTEXPR + +#if defined(BOOST_IF_CONSTEXPR) #define BOND_IF_CONSTEXPR BOOST_IF_CONSTEXPR +#else +#define BOND_IF_CONSTEXPR if +#endif #define BOND_LIB_TYPE_HEADER 1 #define BOND_LIB_TYPE_STATIC 2 From 5310764e306dc788a19720603a504f2924429b69 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Tue, 1 Oct 2024 15:03:31 +0200 Subject: [PATCH 5/6] Translating exception types in capped_allocator tests. --- cpp/test/core/capped_allocator_tests.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/cpp/test/core/capped_allocator_tests.cpp b/cpp/test/core/capped_allocator_tests.cpp index 103d264daf..74b3cda06a 100644 --- a/cpp/test/core/capped_allocator_tests.cpp +++ b/cpp/test/core/capped_allocator_tests.cpp @@ -26,6 +26,9 @@ #include #include +#define TRANSLATE_EXCEPTION(S,F,T) { try { S; } \ + catch(const F&) { throw new T(); } } + BOOST_AUTO_TEST_SUITE(CappedAllocatorTests) using all_counter_types = boost::mpl::list< @@ -338,7 +341,12 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(BondStructDeserializationTest, Reader, all_protoco decltype(alloc) new_alloc{ obj_mem_usage - 1 }; decltype(from) to{ new_alloc }; - BOOST_CHECK_EXCEPTION(bond::Deserialize(reader, to), std::exception, expected_exception); + + // We expect a std::bad_alloc or std::length_error, but this macro can check only against + // a single exception type. We catch length_error and rethrow as bad_alloc as a workaround. + BOOST_CHECK_THROW( + TRANSLATE_EXCEPTION(bond::Deserialize(reader, to), std::length_error, std::bad_alloc), + std::bad_alloc); } // BOOST_TEST_CONTEXT("Runtime schema deserialize without overflow") @@ -359,7 +367,12 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(BondStructDeserializationTest, Reader, all_protoco decltype(alloc) new_alloc{ obj_mem_usage - 1 }; decltype(from) to{ new_alloc }; - BOOST_CHECK_EXCEPTION(bonded.Deserialize(to), std::exception, expected_exception); + + // We expect a std::bad_alloc or std::length_error, but this macro can check only against + // a single exception type. We catch length_error and rethrow as bad_alloc as a workaround. + BOOST_CHECK_THROW( + TRANSLATE_EXCEPTION(bonded.Deserialize(to), std::length_error, std::bad_alloc), + std::bad_alloc); } } From f2cfb4aca21ee6471eae203642baabc5544c7d32 Mon Sep 17 00:00:00 2001 From: Jan Dupej Date: Wed, 2 Oct 2024 15:27:12 +0200 Subject: [PATCH 6/6] Updating change log with msvc 14.0 information. --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f02e12ca1..0e59c2b54e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,10 @@ different versioning scheme, following the Haskell community's any memory is allocated by Bond. * `bond::CoreException` is thrown when the payload has a greater declared size than the backing buffer. +* **Known issue**: Debug builds with MSVC 14.0 (Visual Studio 2015) may fail at + runtime if custom allocators for containers are used. Newer MSVC versions and + other compilers are not affected, neither are Release builds with MSVC 14.0. This + can be worked around by using newer MSVC version or building in Release configuration. ### C# ###