From 1c69169e9e1811dccd6c54c532fedda300233968 Mon Sep 17 00:00:00 2001
From: Sharad Boni <sharadboni@gmail.com>
Date: Wed, 15 Apr 2026 15:38:51 -0700
Subject: [PATCH] Fix integer wraparound in Buffer bounds checks on 32-bit
 platforms

The typed Read methods (ReadU8, ReadU16, ReadU24, ReadU32, ReadTag,
ReadR64) used bounds checks of the form `offset_ + N > length_`. On
32-bit platforms where size_t is 32 bits, if offset_ is close to
SIZE_MAX (e.g. 0xFFFFFFFF), the addition `offset_ + N` wraps around
to a small value, bypassing the bounds check and allowing out-of-bounds
reads from the underlying buffer.

Fix by restructuring the checks to `length_ < N || offset_ > length_ - N`,
which avoids any arithmetic that could overflow. This is consistent with
the existing Read(uint8_t*, size_t) method which already uses the safe
`offset_ > length_ - n_bytes` pattern.

Also add bounds validation to set_offset() (changing return type from
void to bool) to reject offsets beyond the buffer length, and update
the sole caller in font.cc to check the return value. This prevents
TTC font collection offsets from positioning the buffer past its end.
---
 src/buffer.h | 20 +++++++++++++-------
 src/font.cc  |  4 +++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/src/buffer.h b/src/buffer.h
index 7240e51..f9199c7 100644
--- a/src/buffer.h
+++ b/src/buffer.h
@@ -82,7 +82,7 @@ class Buffer {
   }
 
   inline bool ReadU8(uint8_t *value) {
-    if (offset_ + 1 > length_) {
+    if (length_ < 1 || offset_ > length_ - 1) {
       return FONT_COMPRESSION_FAILURE();
     }
     *value = buffer_[offset_];
@@ -91,7 +91,7 @@ class Buffer {
   }
 
   bool ReadU16(uint16_t *value) {
-    if (offset_ + 2 > length_) {
+    if (length_ < 2 || offset_ > length_ - 2) {
       return FONT_COMPRESSION_FAILURE();
     }
     std::memcpy(value, buffer_ + offset_, sizeof(uint16_t));
@@ -105,7 +105,7 @@ class Buffer {
   }
 
   bool ReadU24(uint32_t *value) {
-    if (offset_ + 3 > length_) {
+    if (length_ < 3 || offset_ > length_ - 3) {
       return FONT_COMPRESSION_FAILURE();
     }
     *value = static_cast<uint32_t>(buffer_[offset_]) << 16 |
@@ -116,7 +116,7 @@ class Buffer {
   }
 
   bool ReadU32(uint32_t *value) {
-    if (offset_ + 4 > length_) {
+    if (length_ < 4 || offset_ > length_ - 4) {
       return FONT_COMPRESSION_FAILURE();
     }
     std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
@@ -130,7 +130,7 @@ class Buffer {
   }
 
   bool ReadTag(uint32_t *value) {
-    if (offset_ + 4 > length_) {
+    if (length_ < 4 || offset_ > length_ - 4) {
       return FONT_COMPRESSION_FAILURE();
     }
     std::memcpy(value, buffer_ + offset_, sizeof(uint32_t));
@@ -139,7 +139,7 @@ class Buffer {
   }
 
   bool ReadR64(uint64_t *value) {
-    if (offset_ + 8 > length_) {
+    if (length_ < 8 || offset_ > length_ - 8) {
       return FONT_COMPRESSION_FAILURE();
     }
     std::memcpy(value, buffer_ + offset_, sizeof(uint64_t));
@@ -151,7 +151,13 @@ class Buffer {
   size_t offset() const { return offset_; }
   size_t length() const { return length_; }
 
-  void set_offset(size_t newoffset) { offset_ = newoffset; }
+  bool set_offset(size_t newoffset) {
+    if (newoffset > length_) {
+      return FONT_COMPRESSION_FAILURE();
+    }
+    offset_ = newoffset;
+    return true;
+  }
 
  private:
   const uint8_t * const buffer_;
diff --git a/src/font.cc b/src/font.cc
index a45153e..dcef984 100644
--- a/src/font.cc
+++ b/src/font.cc
@@ -155,7 +155,9 @@ bool ReadTrueTypeCollection(Buffer* file, const uint8_t* data, size_t len,
 
     std::map<uint32_t, Font::Table*> all_tables;
     for (const auto offset : offsets) {
-      file->set_offset(offset);
+      if (!file->set_offset(offset)) {
+        return FONT_COMPRESSION_FAILURE();
+      }
       Font& font = *font_it++;
       if (!ReadCollectionFont(file, data, len, &font, &all_tables)) {
         return FONT_COMPRESSION_FAILURE();