From 3187cede13cd6b4c4d4137a199fee683961de9b4 Mon Sep 17 00:00:00 2001
From: Zhuo Wang <wngzhuo@gmail.com>
Date: Thu, 24 Sep 2026 17:35:40 +0800
Subject: [PATCH] fix(bucket): reject oversized input in BucketUtils::HashBytes

HashBytes passed a size_t length straight to the vendored
MurmurHash3_x86_32, whose length parameter is a signed 32-bit int. For
inputs of 2^31 bytes or more the length wrapped negative, making the
hash loop read far out of bounds (and nblocks * 4 signed-overflow UB);
for inputs above 2^32 bytes a silently wrong prefix was hashed, breaking
the bucket contract against other implementations.

Every literal hash path (kDecimal/kString/kUuid/kBinary/kFixed) funnels
through this one function, so guard it there: reject lengths above
INT32_MAX with ICEBERG_CHECK_OR_DIE before the narrowing cast. The bound
matches the limit implied by iceberg-java's byte arrays, so no legal
bucket input is rejected. The vendored murmurhash3_internal.cc is left
untouched (tagged third-party code) and the now-checked narrowing is
made explicit with static_cast<int>.

Add BucketUtilsTest.HashBytesRejectsOversizedInput, which fabricates an
oversized span over a 1-byte buffer (never dereferenced, so no 2 GiB
allocation is needed in CI). Without the guard the test segfaults.
---
 src/iceberg/test/bucket_util_test.cc | 18 ++++++++++++++++++
 src/iceberg/util/bucket_util.cc      | 14 +++++++++++++-
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/src/iceberg/test/bucket_util_test.cc b/src/iceberg/test/bucket_util_test.cc
index bee84a7cb..cc38be5fa 100644
--- a/src/iceberg/test/bucket_util_test.cc
+++ b/src/iceberg/test/bucket_util_test.cc
@@ -20,9 +20,13 @@
 #include "iceberg/util/bucket_util.h"
 
 #include <chrono>
+#include <cstdint>
+#include <limits>
 
+#include <gmock/gmock.h>
 #include <gtest/gtest.h>
 
+#include "iceberg/exception.h"
 #include "iceberg/expression/literal.h"
 #include "iceberg/test/temporal_test_helper.h"
 #include "iceberg/util/decimal.h"
@@ -108,6 +112,20 @@ TEST(BucketUtilsTest, HashHelper) {
   EXPECT_EQ(BucketUtils::HashBytes(fixed), -188683207);
 }
 
+TEST(BucketUtilsTest, HashBytesRejectsOversizedInput) {
+  // Lengths above INT32_MAX would wrap the 32-bit signed length parameter of
+  // MurmurHash3_x86_32 and read out of bounds. Fabricate an oversized span
+  // (never dereferenced: HashBytes must reject it before touching the data)
+  // to avoid allocating 2 GiB in the test.
+  const uint8_t byte = 0;
+  const auto oversized_length =
+      static_cast<size_t>(std::numeric_limits<int32_t>::max()) + 1;
+  std::span<const uint8_t> oversized(&byte, oversized_length);
+  ASSERT_THAT([&]() { BucketUtils::HashBytes(oversized); },
+              ::testing::ThrowsMessage<IcebergError>(
+                  ::testing::HasSubstr("exceeds the maximum supported length")));
+}
+
 TEST(BucketUtilsTest, BucketTimestampNanosMatchesMicros) {
   constexpr int32_t kNumBuckets = 1000;
   const auto ts_micros = TemporalTestHelper::CreateTimestamp({.year = 2017,
diff --git a/src/iceberg/util/bucket_util.cc b/src/iceberg/util/bucket_util.cc
index 2c0718f5d..ee7dacfd4 100644
--- a/src/iceberg/util/bucket_util.cc
+++ b/src/iceberg/util/bucket_util.cc
@@ -19,8 +19,10 @@
 
 #include "iceberg/util/bucket_util.h"
 
+#include <limits>
 #include <utility>
 
+#include "iceberg/exception.h"
 #include "iceberg/expression/literal.h"
 #include "iceberg/util/endian.h"
 #include "iceberg/util/murmurhash3_internal.h"
@@ -110,8 +112,18 @@ int32_t HashLiteral<TypeId::kFixed>(const Literal& literal) {
 }  // namespace
 
 int32_t BucketUtils::HashBytes(std::span<const uint8_t> bytes) {
+  // MurmurHash3_x86_32 takes a signed 32-bit length. Passing a larger size_t
+  // would wrap it negative and make the hash loop read far outside `bytes`
+  // (and silently hash the wrong prefix for even larger inputs). Iceberg's
+  // bucket transform is defined on byte sequences of at most 2^31 - 1 bytes
+  // (Java arrays), so reject anything longer before narrowing.
+  ICEBERG_CHECK_OR_DIE(
+      bytes.size() <= static_cast<size_t>(std::numeric_limits<int32_t>::max()),
+      "BucketUtils::HashBytes: input length {} exceeds the maximum supported "
+      "length {}",
+      bytes.size(), std::numeric_limits<int32_t>::max());
   int32_t hash_value = 0;
-  MurmurHash3_x86_32(bytes.data(), bytes.size(), 0, &hash_value);
+  MurmurHash3_x86_32(bytes.data(), static_cast<int>(bytes.size()), 0, &hash_value);
   return hash_value;
 }