[c++] Check for memory allocation overflow errors

Detect overflow errors related to memory allocation and memory access,
specifically in: bond::OutputMemoryStream, bond::blob, and
bond::detail::SimpleArray.

CHANGELOG.md

@@ -30,6 +30,7 @@ different versioning scheme, following the Haskell community's
 * Errors from some versions of G++ like "non-template type `Deserialize`
   used as a template" have been fixed.
   [Issue #538](https://github.com/Microsoft/bond/issues/538)
+* Guard against overflows in OutputMemoryStream, blob, and SimpleArray.
 
 ### C# ###
 

cpp/inc/bond/core/blob.h

@@ -6,6 +6,7 @@
 
 #include "config.h"
 #include "container_interface.h"
+#include "detail/checked_add.h"
 #include <boost/shared_array.hpp>
 #include <boost/make_shared.hpp>
 #include <stdint.h>
@@ -38,6 +39,7 @@ public:
           _content(static_cast<const char*>(content)),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a boost::shared_ptr to const memory buffer
@@ -46,14 +48,16 @@ public:
           _content(_buffer.get()),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a boost::shared_ptr to const memory buffer
     blob(const boost::shared_ptr<const char[]>& buffer, uint32_t offset, uint32_t length)
         : _buffer(buffer),
-          _content(_buffer.get() + offset),
+          _content(bond::detail::checked_add(_buffer.get(), offset)),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a boost::shared_ptr to memory buffer
@@ -62,14 +66,16 @@ public:
           _content(_buffer.get()),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a boost::shared_ptr to memory buffer
     blob(const boost::shared_ptr<char[]>& buffer, uint32_t offset, uint32_t length)
         : _buffer(buffer),
-          _content(_buffer.get() + offset),
+          _content(bond::detail::checked_add(_buffer.get(), offset)),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a smart pointer other than boost::shared_ptr
@@ -81,6 +87,7 @@ public:
           _content(_buffer.get()),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
     /// @brief Construct from a smart pointer other than boost::shared_ptr
@@ -89,9 +96,10 @@ public:
     template <typename T, template <typename U> class SmartPtr>
     blob(const SmartPtr<T>& buffer, uint32_t offset, uint32_t length)
         : _buffer(wrap_in_shared_ptr(buffer)),
-          _content(_buffer.get() + offset),
+          _content(bond::detail::checked_add(_buffer.get(), offset)),
           _length(length)
     {
+        bond::detail::checked_add(_content, length);
     }
 
 #ifndef BOND_NO_CXX11_RVALUE_REFERENCES
@@ -115,7 +123,10 @@ public:
     /// @brief Assign a new value from another blob object or its part
     void assign(const blob& from, uint32_t offset, uint32_t length)
     {
-        BOOST_ASSERT((offset + length) <= from._length);
+        if (bond::detail::checked_add(offset, length) > from._length)
+        {
+            throw std::invalid_argument("Total of offset and length too large; must be less than or equal to length of blob");
+        }
 
         _buffer = from._buffer;
         _content = from._content + offset;
@@ -143,7 +154,10 @@ public:
     /// @brief Return a blob object for a range of this object
     blob range(uint32_t offset, uint32_t length) const
     {
-        BOOST_ASSERT((offset + length) <= _length);
+        if (bond::detail::checked_add(offset, length) > _length)
+        {
+            throw std::invalid_argument("Total of offset and length too large; must be less than or equal to length of blob");
+        }
 
         blob temp;
         temp._buffer = _buffer;
@@ -157,7 +171,10 @@ public:
     /// the end of the buffer
     blob range(uint32_t offset) const
     {
-        BOOST_ASSERT(offset <= _length);
+        if (offset > _length)
+        {
+            throw std::invalid_argument("Offset too large; must be less than or equal to length of blob");
+        }
 
         blob temp = *this;
         temp._content += offset;
@@ -291,7 +308,7 @@ inline blob merge(const A& allocator, const blob& x, const blob& y)
     }
     else
     {
-        uint32_t length = x.length() + y.length();
+        uint32_t length = detail::checked_add(x.length(), y.length());
         boost::shared_ptr<char[]> buffer = boost::allocate_shared_noinit<char[]>(allocator, length);
 
         ::memcpy(buffer.get(), x.content(), x.length());
@@ -310,7 +327,7 @@ inline blob merge(const A& allocator, t_It begin, t_It end)
     uint32_t length = 0;
     for (t_It it = begin; it != end; ++it)
     {
-        length += it->length();
+        length = detail::checked_add(length, it->length());
     }
 
     if (0 == length)
@@ -320,8 +337,7 @@ inline blob merge(const A& allocator, t_It begin, t_It end)
         //
         return blob();
     }
-    else
-    if (length == begin->length())
+    else if (length == begin->length())
     {
         //
         // just first blob in the sequence is not empty

cpp/inc/bond/core/detail/checked_add.h

@@ -0,0 +1,47 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+#pragma once
+
+#include <cstdint>
+#include <limits>
+#include <stdexcept>
+#include <boost/static_assert.hpp>
+#include <boost/type_traits.hpp>
+#include <boost/utility.hpp>
+
+namespace bond
+{
+namespace detail
+{
+
+    inline const char* checked_add(const char* ptr, uint32_t offset)
+    {
+        // Numeric limit logic below requires uintptr_t to be exactly same size as a pointer
+        BOOST_STATIC_ASSERT(sizeof(const char*) == sizeof(std::uintptr_t));
+
+        std::uintptr_t uintptr = reinterpret_cast<std::uintptr_t>(ptr);
+        if (((std::numeric_limits<uintptr_t>::max)() - offset) < uintptr)
+        {
+            throw std::overflow_error("Offset caused pointer to overflow");
+        }
+        return ptr + offset;
+    }
+
+    template <typename T, typename U>
+    typename boost::enable_if_c<boost::is_integral<T>::value && boost::is_unsigned<T>::value
+                                && boost::is_integral<U>::value && boost::is_unsigned<U>::value, T>::type
+    checked_add(T lhs, U rhs)
+    {
+        BOOST_STATIC_ASSERT(sizeof(T) >= sizeof(U));
+
+        if (((std::numeric_limits<T>::max)() - lhs) < rhs)
+        {
+            throw std::overflow_error("Overflow on addition");
+        }
+        return lhs + rhs;
+    }
+
+} // namespace detail
+} // namespace bond
+

cpp/inc/bond/protocol/detail/simple_array.h

@@ -3,6 +3,9 @@
 
 #pragma once
 
+#include <limits>
+#include <stdexcept>
+
 namespace bond
 {
 namespace detail
@@ -54,6 +57,12 @@ public:
 private:
     void grow(T x)
     {
+        // cap elements to prevent overflow
+        if (_capacity >= ((std::numeric_limits<uint32_t>::max)() >> 1))
+        {
+            throw std::bad_alloc();
+        }
+
         T* new_data = new T[_capacity <<= 1];
         memcpy(new_data, _data, _size * sizeof(T));
         memfree();

cpp/inc/bond/stream/output_buffer.h

@@ -7,8 +7,11 @@
 #include <bond/core/blob.h>
 #include <bond/core/containers.h>
 #include <bond/core/traits.h>
+#include <bond/core/detail/checked_add.h>
 #include <boost/static_assert.hpp>
 #include <cstring>
+#include <limits>
+#include <stdexcept>
 
 namespace bond
 {
@@ -143,7 +146,7 @@ public:
     template <typename T>
     void GetBuffers(std::vector<blob, T>& buffers) const
     {
-        buffers.reserve(_blobs.size() + 1);
+        buffers.reserve(bond::detail::checked_add(_blobs.size(), 1U));
 
         //
         // insert all "ready" blobs
@@ -246,6 +249,12 @@ public:
                 _blobs.push_back(blob(_buffer, _rangeOffset, _rangeSize));
             }
 
+            // cap buffer to prevent overflow
+            if (_bufferSize > ((std::numeric_limits<uint32_t>::max)() >> 1))
+            {
+                throw std::bad_alloc();
+            }
+
             //
             // grow buffer by 50% (at least 4096 bytes for initial buffer)
             // and enough to store left overs of specified buffer

cpp/test/core/CMakeLists.txt

@@ -92,6 +92,7 @@ add_unit_test (basic_type_lists.cpp)
 add_unit_test (basic_type_map.cpp)
 add_unit_test (blob_tests.cpp)
 add_unit_test (bonded_tests.cpp)
+add_unit_test (checked_add_test.cpp)
 add_unit_test (cmdargs.cpp)
 add_unit_test (container_extensibility.cpp
     associative_container_extensibility.cpp)

cpp/test/core/checked_add_test.cpp

@@ -0,0 +1,69 @@
+#include "precompiled.h"
+
+#include <bond/core/detail/checked_add.h>
+
+struct CheckedAddUIntTest
+{
+    template <typename X>
+    void operator()(const X&)
+    {
+        X x1 = (std::numeric_limits<X>::max)();
+        X x2 = (std::numeric_limits<X>::max)() - 50;
+        X x3 = 1;
+        X x4 = 100;
+
+        UT_AssertThrows(bond::detail::checked_add(x1, x2), std::overflow_error);
+        UT_AssertThrows(bond::detail::checked_add(x1, x3), std::overflow_error);
+        UT_AssertThrows(bond::detail::checked_add(x1, x4), std::overflow_error);
+        UT_AssertAreEqual(bond::detail::checked_add(x2, x3), x2 + x3);
+        UT_AssertThrows(bond::detail::checked_add(x2, x4), std::overflow_error);
+        UT_AssertAreEqual(bond::detail::checked_add(x3, x4), x3 + x4);
+    }
+};
+
+TEST_CASE_BEGIN(CheckedAddUIntTests)
+{
+    typedef boost::mpl::list<
+        // int8_t, // Signed types will fail compilation
+        // float,  // Non-integer types will fail compilation
+        uint8_t,
+        uint16_t,
+        uint32_t,
+        uint64_t
+    > Types;
+
+    boost::mpl::for_each<Types>(CheckedAddUIntTest());
+}
+TEST_CASE_END
+
+TEST_CASE_BEGIN(CheckedAddPtrTests)
+{
+    const char* max = reinterpret_cast<const char*>((std::numeric_limits<std::uintptr_t>::max)());
+    const char* close = reinterpret_cast<const char*>((std::numeric_limits<std::uintptr_t>::max)() - 50);
+    const char* zero = reinterpret_cast<const char*>(0);
+
+    uint32_t one = 1;
+    uint32_t hundred = 100;
+
+    UT_AssertThrows(bond::detail::checked_add(max, one), std::overflow_error);
+    UT_AssertThrows(bond::detail::checked_add(max, hundred), std::overflow_error);
+    UT_AssertIsTrue(bond::detail::checked_add(close, one) == (close + one));
+    UT_AssertThrows(bond::detail::checked_add(close, hundred), std::overflow_error);
+    UT_AssertIsTrue(bond::detail::checked_add(zero, one) == (zero + one));
+    UT_AssertIsTrue(bond::detail::checked_add(zero, hundred) == (zero + hundred));
+}
+TEST_CASE_END
+
+void Initialize()
+{
+    UnitTestSuite suite("checked_add tests");
+    AddTestCase<TEST_ID(0x2501), CheckedAddUIntTests>(suite, "unsigned integer checked_add tests");
+    AddTestCase<TEST_ID(0x2502), CheckedAddPtrTests>(suite, "pointer checked_add tests");
+}
+
+bool init_unit_test()
+{
+    Initialize();
+    return true;
+}
+

cpp/test/core/checked_add_test.h

@@ -0,0 +1,7 @@
+#pragma once
+
+class ValidateTest
+{
+public:
+    static void Initialize();
+};