Hardening: add null guards, bounds checks, and return correctness to allocators, vector iterators, and stl emulation

include/flatbuffers/allocator.h

@@ -43,6 +43,7 @@ class Allocator {
                                        size_t in_use_front) {
     FLATBUFFERS_ASSERT(new_size > old_size);  // vector_downward only grows
     uint8_t* new_p = allocate(new_size);
+    if (!new_p) return nullptr;
     memcpy_downward(old_p, old_size, new_p, new_size, in_use_back,
                     in_use_front);
     deallocate(old_p, old_size);
@@ -57,9 +58,13 @@ class Allocator {
   void memcpy_downward(uint8_t* old_p, size_t old_size, uint8_t* new_p,
                        size_t new_size, size_t in_use_back,
                        size_t in_use_front) {
-    memcpy(new_p + new_size - in_use_back, old_p + old_size - in_use_back,
-           in_use_back);
-    memcpy(new_p, old_p, in_use_front);
+    if (in_use_back > 0 && new_p && old_p) {
+      memcpy(new_p + new_size - in_use_back, old_p + old_size - in_use_back,
+             in_use_back);
+    }
+    if (in_use_front > 0 && new_p && old_p) {
+      memcpy(new_p, old_p, in_use_front);
+    }
   }
 };
 

include/flatbuffers/default_allocator.h

@@ -26,12 +26,16 @@ namespace flatbuffers {
 class DefaultAllocator : public Allocator {
  public:
   uint8_t* allocate(size_t size) FLATBUFFERS_OVERRIDE {
-    return new uint8_t[size];
+    return size ? new uint8_t[size] : nullptr;
   }
 
-  void deallocate(uint8_t* p, size_t) FLATBUFFERS_OVERRIDE { delete[] p; }
+  void deallocate(uint8_t* p, size_t) FLATBUFFERS_OVERRIDE {
+    if (p) delete[] p;
+  }
 
-  static void dealloc(void* p, size_t) { delete[] static_cast<uint8_t*>(p); }
+  static void dealloc(void* p, size_t) {
+    if (p) delete[] static_cast<uint8_t*>(p);
+  }
 };
 
 // These functions allow for a null allocator to mean use the default allocator,
@@ -44,6 +48,7 @@ inline uint8_t* Allocate(Allocator* allocator, size_t size) {
 }
 
 inline void Deallocate(Allocator* allocator, uint8_t* p, size_t size) {
+  if (!p) return;
   if (allocator)
     allocator->deallocate(p, size);
   else

include/flatbuffers/stl_emulation.h

@@ -238,11 +238,28 @@ class Optional FLATBUFFERS_FINAL_CLASS {
     return value_;
   }
 
+  FLATBUFFERS_CONSTEXPR_CPP14 T& operator*() FLATBUFFERS_NOEXCEPT {
+    return value_;
+  }
+
+  const T* operator->() const FLATBUFFERS_NOEXCEPT {
+    return &value_;
+  }
+
+  T* operator->() FLATBUFFERS_NOEXCEPT {
+    return &value_;
+  }
+
   const T& value() const {
     FLATBUFFERS_ASSERT(has_value());
     return value_;
   }
 
+  T& value() {
+    FLATBUFFERS_ASSERT(has_value());
+    return value_;
+  }
+
   T value_or(T default_value) const FLATBUFFERS_NOEXCEPT {
     return has_value() ? value_ : default_value;
   }
@@ -391,6 +408,7 @@ class span FLATBUFFERS_FINAL_CLASS {
       FLATBUFFERS_NOEXCEPT {
     data_ = other.data_;
     count_ = other.count_;
+    return *this;
   }
 
   // Limited implementation of
@@ -468,7 +486,7 @@ class span FLATBUFFERS_FINAL_CLASS {
 
  private:
   // This is a naive implementation with 'count_' member even if (Extent != dynamic_extent).
-  pointer const data_;
+  pointer data_;
   size_type count_;
 };
 #endif  // defined(FLATBUFFERS_USE_STD_SPAN)

include/flatbuffers/vector.h

@@ -38,7 +38,8 @@ struct VectorIterator {
 
   static const SizeT element_stride = IndirectHelper<T>::element_stride;
 
-  VectorIterator(Data data, SizeT i) : data_(data + element_stride * i) {}
+  VectorIterator(Data data, SizeT i)
+      : data_((data != nullptr) ? (data + element_stride * i) : nullptr) {}
   VectorIterator(const VectorIterator& other) : data_(other.data_) {}
   VectorIterator() : data_(nullptr) {}
 
@@ -77,7 +78,7 @@ struct VectorIterator {
   }
 
   difference_type operator-(const VectorIterator& other) const {
-    return (data_ - other.data_) / element_stride;
+    return element_stride != 0 ? ((data_ - other.data_) / element_stride) : 0;
   }
 
   // Note: return type is incompatible with the standard
@@ -89,42 +90,44 @@ struct VectorIterator {
   IT operator->() const { return IndirectHelper<T>::Read(data_, 0); }
 
   VectorIterator& operator++() {
-    data_ += element_stride;
+    if (data_ != nullptr) data_ += element_stride;
     return *this;
   }
 
   VectorIterator operator++(int) {
     VectorIterator temp(data_, 0);
-    data_ += element_stride;
+    if (data_ != nullptr) data_ += element_stride;
     return temp;
   }
 
   VectorIterator operator+(const SizeT& offset) const {
-    return VectorIterator(data_ + offset * element_stride, 0);
+    return VectorIterator(
+        (data_ != nullptr) ? (data_ + offset * element_stride) : nullptr, 0);
   }
 
   VectorIterator& operator+=(const SizeT& offset) {
-    data_ += offset * element_stride;
+    if (data_ != nullptr) data_ += offset * element_stride;
     return *this;
   }
 
   VectorIterator& operator--() {
-    data_ -= element_stride;
+    if (data_ != nullptr) data_ -= element_stride;
     return *this;
   }
 
   VectorIterator operator--(int) {
     VectorIterator temp(data_, 0);
-    data_ -= element_stride;
+    if (data_ != nullptr) data_ -= element_stride;
     return temp;
   }
 
   VectorIterator operator-(const SizeT& offset) const {
-    return VectorIterator(data_ - offset * element_stride, 0);
+    return VectorIterator(
+        (data_ != nullptr) ? (data_ - offset * element_stride) : nullptr, 0);
   }
 
   VectorIterator& operator-=(const SizeT& offset) {
-    data_ -= offset * element_stride;
+    if (data_ != nullptr) data_ -= offset * element_stride;
     return *this;
   }
 
@@ -283,6 +286,9 @@ class Vector {
 
   template <typename K>
   return_type LookupByKey(K key) const {
+    if (!size()) {
+      return nullptr;
+    }
     void* search_result = std::bsearch(
         &key, Data(), size(), IndirectHelper<T>::element_stride, KeyCompare<K>);
 
@@ -317,7 +323,9 @@ class Vector {
   static int KeyCompare(const void* ap, const void* bp) {
     const K* key = reinterpret_cast<const K*>(ap);
     const uint8_t* data = reinterpret_cast<const uint8_t*>(bp);
+    if (!key || !data) return 0;
     auto table = IndirectHelper<T>::Read(data, 0);
+    if (!table) return 0;
 
     // std::bsearch compares with the operands transposed, so we negate the
     // result here.
@@ -399,13 +407,13 @@ FLATBUFFERS_CONSTEXPR_CPP11 flatbuffers::span<const U> make_span(
 template <class U>
 FLATBUFFERS_CONSTEXPR_CPP11 flatbuffers::span<U> make_structs_span(
     Vector<const U*>* ptr) FLATBUFFERS_NOEXCEPT {
-  return ptr ? make_span(*ptr) : span<U>();
+  return ptr ? make_structs_span(*ptr) : span<U>();
 }
 
 template <class U>
 FLATBUFFERS_CONSTEXPR_CPP11 flatbuffers::span<const U> make_structs_span(
     const Vector<const U*>* ptr) FLATBUFFERS_NOEXCEPT {
-  return ptr ? make_span(*ptr) : span<const U>();
+  return ptr ? make_structs_span(*ptr) : span<const U>();
 }
 
 #endif
@@ -434,13 +442,13 @@ class VectorOfAny {
 template <typename T, typename U>
 Vector<Offset<T>>* VectorCast(Vector<Offset<U>>* ptr) {
   static_assert(std::is_base_of<T, U>::value, "Unrelated types");
-  return reinterpret_cast<Vector<Offset<T>>*>(ptr);
+  return ptr ? reinterpret_cast<Vector<Offset<T>>*>(ptr) : nullptr;
 }
 
 template <typename T, typename U>
 const Vector<Offset<T>>* VectorCast(const Vector<Offset<U>>* ptr) {
   static_assert(std::is_base_of<T, U>::value, "Unrelated types");
-  return reinterpret_cast<const Vector<Offset<T>>*>(ptr);
+  return ptr ? reinterpret_cast<const Vector<Offset<T>>*>(ptr) : nullptr;
 }
 
 // Convenient helper function to get the length of any vector, regardless

tests/test.cpp

@@ -1745,6 +1745,52 @@ void CrossNamespacePackTest() {
   TEST_EQ(unpacked->c2[0]->value, 99);
 }
 
+void StlEmulationAndVectorSafetyTest() {
+  // Test Optional mutable operator*, value()
+  flatbuffers::Optional<int> opt_int(123);
+  TEST_EQ(opt_int.has_value(), true);
+  TEST_EQ(*opt_int, 123);
+  *opt_int = 456;
+  TEST_EQ(opt_int.value(), 456);
+  opt_int.value() = 789;
+  TEST_EQ(*opt_int, 789);
+
+  const flatbuffers::Optional<int> const_opt(55);
+  TEST_EQ(*const_opt, 55);
+  TEST_EQ(const_opt.value(), 55);
+
+  flatbuffers::Optional<int> opt_empty;
+  TEST_EQ(opt_empty.has_value(), false);
+  TEST_EQ(opt_empty.value_or(42), 42);
+
+  // Test span copy-assignment and return statement
+  int arr1[] = {1, 2, 3};
+  int arr2[] = {4, 5, 6};
+  flatbuffers::span<int> s1(arr1, 3);
+  flatbuffers::span<int> s2(arr2, 3);
+  s1 = s2;
+  TEST_EQ(s1.size(), 3);
+  TEST_EQ(s1[0], 4);
+
+  // Test VectorIterator default and null handling
+  flatbuffers::VectorConstIterator<uint8_t, uint8_t> it1;
+  flatbuffers::VectorConstIterator<uint8_t, uint8_t> it2;
+  TEST_EQ(it1 == it2, true);
+  TEST_EQ(it1 - it2, 0);
+
+  // Test VectorCast null safety
+  flatbuffers::Vector<flatbuffers::Offset<flatbuffers::String>>* null_vec = nullptr;
+  auto cast_vec = flatbuffers::VectorCast<flatbuffers::String, flatbuffers::String>(null_vec);
+  TEST_NULL(cast_vec);
+
+  // Test Allocator null handling
+  flatbuffers::DefaultAllocator alloc;
+  uint8_t* p = alloc.allocate(0);
+  TEST_NULL(p);
+  alloc.deallocate(nullptr, 0);
+  flatbuffers::Deallocate(nullptr, nullptr, 0);
+}
+
 int FlatBufferTests(const std::string& tests_data_path) {
   // Run our various test suites:
 
@@ -1864,6 +1910,7 @@ int FlatBufferTests(const std::string& tests_data_path) {
   StructsInHashTableTest();
   DefaultVectorsStringsTest();
   CrossNamespacePackTest();
+  StlEmulationAndVectorSafetyTest();
   return 0;
 }
 }  // namespace