(iceberg-cpp) branch main updated: fix(deletes): validate roaring bitmap after deserialization (#1005)

[gangwu]

This is an automated email from the ASF dual-hosted git repository.

wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git


The following commit(s) were added to refs/heads/main by this push:
     new d98d8518 fix(deletes): validate roaring bitmap after deserialization 
(#1005)
d98d8518 is described below

commit d98d851873bd6bd9a456ae7f58fb430da2bceb6e
Author: Guotao Yu <guotao.y...@gmail.com>
AuthorDate: Thu Oct 15 15:00:12 2026 +0800

    fix(deletes): validate roaring bitmap after deserialization (#1005)
    
    fix(deletes): validate roaring bitmap structure after deserialization
    
    readSafe only bounds-checks the read and does not verify CRoaring's
    structural invariants. A forged deletion vector (e.g. bitset container
    with a wrong cardinality or wrapping run containers) could later cause
    out-of-bounds access during merge or serialize. Call
    roaring_bitmap_internal_validate right after readSafe and reject invalid
    bitmaps with InvalidArgument.
---
 src/iceberg/deletes/roaring_position_bitmap.cc   | 11 +++++++++++
 src/iceberg/test/roaring_position_bitmap_test.cc | 25 ++++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/iceberg/deletes/roaring_position_bitmap.cc 
b/src/iceberg/deletes/roaring_position_bitmap.cc
index a2827d4b..e043d764 100644
--- a/src/iceberg/deletes/roaring_position_bitmap.cc
+++ b/src/iceberg/deletes/roaring_position_bitmap.cc
@@ -258,6 +258,17 @@ Result<RoaringPositionBitmap> 
RoaringPositionBitmap::Deserialize(std::string_vie
     } catch (const std::exception& e) {
       return InvalidArgument("Failed to deserialize bitmap at key {}: {}", 
key, e.what());
     }
+
+    // readSafe does not verify structural invariants; an invalid bitmap can 
cause
+    // out-of-bounds access in later merge/serialize, so validate here.
+    const char* invalid_reason = nullptr;
+    if (!roaring::api::roaring_bitmap_internal_validate(&bitmap.roaring,
+                                                        &invalid_reason)) {
+      return InvalidArgument(
+          "Invalid bitmap at key {}: {}", key,
+          invalid_reason != nullptr ? invalid_reason : "unknown reason");
+    }
+
     size_t bitmap_size = bitmap.getSizeInBytes(/*portable=*/true);
     ICEBERG_PRECHECK(
         bitmap_size <= remaining,
diff --git a/src/iceberg/test/roaring_position_bitmap_test.cc 
b/src/iceberg/test/roaring_position_bitmap_test.cc
index 8dd998ef..8d29e725 100644
--- a/src/iceberg/test/roaring_position_bitmap_test.cc
+++ b/src/iceberg/test/roaring_position_bitmap_test.cc
@@ -478,6 +478,31 @@ TEST(RoaringPositionBitmapTest, 
TestSerializeDeserializeAllContainerBitmap) {
   AssertEqualContent(copy, expected_positions);
 }
 
+TEST(RoaringPositionBitmapTest, 
TestDeserializeRejectsStructurallyInvalidBitmap) {
+  auto append_le = [](std::string& out, uint64_t value, int bytes) {
+    for (int i = 0; i < bytes; ++i) {
+      out.push_back(static_cast<char>((value >> (8 * i)) & 0xFF));
+    }
+  };
+
+  std::string blob;
+  append_le(blob, 1, 8);  // one bitmap
+  append_le(blob, 0, 4);  // key 0
+
+  // Portable roaring without run containers: one bitset container whose header
+  // claims cardinality 4097 while all 65536 bits are actually set.
+  append_le(blob, 12346, 4);              // SERIAL_COOKIE_NO_RUNCONTAINER
+  append_le(blob, 1, 4);                  // container count
+  append_le(blob, 0, 2);                  // container key
+  append_le(blob, 4096, 2);               // cardinality - 1
+  append_le(blob, 8 + 4 + 4 + 4 + 4, 4);  // data offset (unused by readSafe)
+  blob.append(8192, '\xFF');
+
+  auto result = RoaringPositionBitmap::Deserialize(blob);
+  ASSERT_THAT(result, IsError(ErrorKind::kInvalidArgument));
+  ASSERT_THAT(result, HasErrorMessage("Invalid bitmap at key 0"));
+}
+
 TEST(RoaringPositionBitmapTest, TestForEach) {
   RoaringPositionBitmap bitmap;
   bitmap.Add(30L);