From 1164b20d27c9ac58caea521be07c45fea7c7ce51 Mon Sep 17 00:00:00 2001
From: Sam Rigby <samrigby781@gmail.com>
Date: Wed, 24 Jun 2026 19:27:41 +0100
Subject: [PATCH] fix: add bounds assertions and safety docs for FlexBuffer OOB
 reads
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Three locations in flexbuffers.h can trigger out-of-bounds reads (CWE-125)
when called on unverified input:

1. Sized::read_size() (line ~241): reads `byte_width_` bytes before data_.
   A malformed byte_width_ (not 1/2/4/8) or a data_ pointer too close to
   the buffer start causes a read before the allocation.

2. Map::Keys() (line ~351): accesses data_ - byte_width_*3 for the three
   Map prefix fields. Same root cause as #1.

3. Vector::operator[] (line ~962): uses size() — which calls read_size() —
   as the loop bound, then reads type bytes at data_ + len*byte_width_. A
   corrupt len can push this read arbitrarily past the buffer end.

The FlexBuffer Verifier already catches all three patterns when
flexbuffers::VerifyBuffer() is called before data access (it calls
VerifyBeforePointer for the size prefix and VerifyFromPointer for the
element and type-byte spans). These changes make the contract explicit:

- Add FLATBUFFERS_ASSERT in read_size() and Keys() that byte_width_ is one
  of the four legal values; a corrupt value will now fire in debug builds.
- Add clarifying comments at all three sites referencing the Verifier
  requirement and the CWE number so callers understand the precondition.

Reported by: Sam Rigby (samrigby432@outlook.com)
---
 include/flatbuffers/flexbuffers.h | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/include/flatbuffers/flexbuffers.h b/include/flatbuffers/flexbuffers.h
index 5c42a7ed47..0417049867 100644
--- a/include/flatbuffers/flexbuffers.h
+++ b/include/flatbuffers/flexbuffers.h
@@ -237,7 +237,14 @@ class Sized : public Object {
       : Object(data, byte_width), size_(sz) {}
   size_t size() const { return size_; }
   // Access size stored in `byte_width_` bytes before data_ pointer.
+  // REQUIRES: the buffer containing data_ has been verified with
+  // flexbuffers::VerifyBuffer() before calling this, so that
+  // (data_ - byte_width_) is guaranteed to be within the allocation.
   size_t read_size() const {
+    // byte_width_ must be 1, 2, 4, or 8; values outside this range indicate
+    // a corrupt or unverified buffer and would cause an OOB read below.
+    FLATBUFFERS_ASSERT(byte_width_ == 1 || byte_width_ == 2 ||
+                       byte_width_ == 4 || byte_width_ == 8);
     return static_cast<size_t>(ReadUInt64(data_ - byte_width_, byte_width_));
   }
 
@@ -349,7 +356,16 @@ class Map : public Vector {
   Vector Values() const { return Vector(data_, byte_width_); }
 
   TypedVector Keys() const {
+    // A Map's data_ pointer is preceded by three byte_width_-byte fields:
+    // [keys_offset | keys_byte_width | num_values]. Reading these requires
+    // that (data_ - byte_width_ * 3) lies within the buffer. Callers MUST
+    // verify the buffer with flexbuffers::VerifyBuffer() before calling Keys()
+    // on untrusted data; the FlexBuffer Verifier checks VerifyBeforePointer for
+    // exactly this span.
     const size_t num_prefixed_fields = 3;
+    // Guard against byte_width_ overflow in the subtraction.
+    FLATBUFFERS_ASSERT(byte_width_ == 1 || byte_width_ == 2 ||
+                       byte_width_ == 4 || byte_width_ == 8);
     auto keys_offset = data_ - byte_width_ * num_prefixed_fields;
     return TypedVector(Indirect(keys_offset, byte_width_),
                        static_cast<uint8_t>(
@@ -942,7 +958,11 @@ inline uint8_t NullPackedType() { return PackedType(BIT_WIDTH_8, FBT_NULL); }
 // typed" data, you may not want that (someone sends you a 2d vector and you
 // wanted 3d).
 // The Null converts seamlessly into a default value for any other type.
-// TODO(wvo): Could introduce an #ifdef that makes this into an assert?
+//
+// IMPORTANT: `size()` and the packed-type array at (data_ + len*byte_width_)
+// are only safe to read when the buffer has been verified with
+// flexbuffers::VerifyBuffer(). On unverified data a corrupt `len` value can
+// cause reads far outside the buffer allocation (CWE-125).
 inline Reference Vector::operator[](size_t i) const {
   auto len = size();
   if (i >= len) return Reference(nullptr, 1, NullPackedType());