From 4be8f6ff334c556343664250315d3c82c27d9206 Mon Sep 17 00:00:00 2001
From: SongT-50 <higheun@gmail.com>
Date: Fri, 1 May 2026 17:13:56 +0900
Subject: [PATCH] Add NDEBUG-safe boundary checks in SnappyIOVecReader::Advance

The current Advance() relies on a debug-only assert to enforce the invariant
total_size_remaining_ >= curr_size_remaining_. Under NDEBUG (most production
builds) the assert is stripped, leaving the inner do-while loop unguarded
against caller-misuse of the raw API RawCompressFromIOVec(iov,
uncompressed_length, ...) when uncompressed_length does not match the actual
sum of iov_len values.

Two defensive checks are added (interim, NDEBUG-safe):

 1. If total_size_remaining_ < curr_size_remaining_ on entry, saturate to
    no-data state and bail out rather than underflowing total_size_remaining_
    into a large unsigned value below.

 2. Cap consecutive zero-length iovec walks at kMaxEmptyWalks (1024). A run
    of zero-length trailing entries with nonzero claimed total_size would
    otherwise spin past the array end via the do-while loop.

The default safe wrapper CompressFromIOVec(iov, iov_cnt, ...) already
computes the total itself before calling the raw API and is unaffected by
this change. The patch is backward compatible (API signature unchanged).

A more permanent API-level fix is to add a safe overload of
RawCompressFromIOVec that takes iov_cnt explicitly and deprecate the current
3-arg form. Happy to prepare a follow-up PR for that direction if
maintainers prefer.

Findings cross-verified through independent code auditing (multi-model code
review pipeline).
---
 snappy.cc | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/snappy.cc b/snappy.cc
index d6d709a..dc9d2ee 100644
--- a/snappy.cc
+++ b/snappy.cc
@@ -1935,7 +1935,24 @@ class SnappyIOVecReader : public Source {
  private:
   // Advances to the next nonempty `iovec` and updates related variables.
   void Advance() {
+    // Best-effort cap on consecutive zero-length iovec walks. Without an
+    // explicit `iov_cnt` parameter (see API-level fix recommendation in the
+    // accompanying PR description), this is the safest interim guard against
+    // a run of zero-length trailing entries paired with a nonzero claimed
+    // `total_size_remaining_`.
+    int empty_walks = 0;
+    constexpr int kMaxEmptyWalks = 1024;
     do {
+      // Defensive check 1 (NDEBUG-safe): caller passed inconsistent
+      // `total_size_remaining_` (greater than the sum of actual `iov_len`
+      // values). Saturate to a no-data state and bail out instead of
+      // underflowing into a large unsigned value below.
+      if (total_size_remaining_ < curr_size_remaining_) {
+        curr_pos_ = nullptr;
+        curr_size_remaining_ = 0;
+        total_size_remaining_ = 0;
+        return;
+      }
       assert(total_size_remaining_ >= curr_size_remaining_);
       total_size_remaining_ -= curr_size_remaining_;
       if (total_size_remaining_ == 0) {
@@ -1943,6 +1960,20 @@ class SnappyIOVecReader : public Source {
         curr_size_remaining_ = 0;
         return;
       }
+      // Defensive check 2: zero-length entries cap. If the caller claims
+      // `total_size_remaining_ > 0` but only zero-length iovecs are
+      // available, the do-while loop would otherwise spin past the array
+      // end. Bail out after `kMaxEmptyWalks` consecutive zero-length steps.
+      if (curr_size_remaining_ == 0) {
+        if (++empty_walks > kMaxEmptyWalks) {
+          curr_pos_ = nullptr;
+          curr_size_remaining_ = 0;
+          total_size_remaining_ = 0;
+          return;
+        }
+      } else {
+        empty_walks = 0;
+      }
       ++curr_iov_;
       curr_pos_ = reinterpret_cast<const char*>(curr_iov_->iov_base);
       curr_size_remaining_ = curr_iov_->iov_len;