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 +#include +#include +#include #include +#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(std::numeric_limits::max()) + 1; + std::span oversized(&byte, oversized_length); + ASSERT_THAT([&]() { BucketUtils::HashBytes(oversized); }, + ::testing::ThrowsMessage( + ::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 #include +#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(const Literal& literal) { } // namespace int32_t BucketUtils::HashBytes(std::span 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(std::numeric_limits::max()), + "BucketUtils::HashBytes: input length {} exceeds the maximum supported " + "length {}", + bytes.size(), std::numeric_limits::max()); int32_t hash_value = 0; - MurmurHash3_x86_32(bytes.data(), bytes.size(), 0, &hash_value); + MurmurHash3_x86_32(bytes.data(), static_cast(bytes.size()), 0, &hash_value); return hash_value; }