You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by ga...@apache.org on 2026/10/15 08:42:45 UTC
(iceberg-cpp) branch main updated: fix(bucket): reject oversized input in BucketUtils::HashBytes (#961)
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 768ac659 fix(bucket): reject oversized input in BucketUtils::HashBytes (#961)
768ac659 is described below
commit 768ac65919ce7d1bc8dc51371f7b87f045d882c2
Author: wzhuo <wn...@gmail.com>
AuthorDate: Thu Oct 15 16:42:39 2026 +0800
fix(bucket): reject oversized input in BucketUtils::HashBytes (#961)
* 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.
* test(bucket): guard raw byte input so the oversized-length test is well-formed
The regression test built a std::span over INT32_MAX + 1 elements of a
one-byte object, which violates the std::span constructor precondition and
is undefined behavior even though the data is never read.
Move the length guard into an internal HashBytes(const uint8_t*, size_t)
overload and keep BucketUtils::HashBytes(std::span) as a thin wrapper, so the
test can pass a null pointer with the oversized length instead.
Also document that HashBytes and BucketIndex throw IcebergError for inputs
larger than INT32_MAX, which the public headers did not mention.
---------
Co-authored-by: Gang Wu <us...@gmail.com>
---
src/iceberg/test/bucket_util_test.cc | 18 +++++++++++++
src/iceberg/util/bucket_util.cc | 23 ++++++++++++++++-
src/iceberg/util/bucket_util.h | 7 +++++
src/iceberg/util/bucket_util_internal.h | 45 +++++++++++++++++++++++++++++++++
4 files changed, 92 insertions(+), 1 deletion(-)
diff --git a/src/iceberg/test/bucket_util_test.cc b/src/iceberg/test/bucket_util_test.cc
index bee84a7c..e75933cf 100644
--- a/src/iceberg/test/bucket_util_test.cc
+++ b/src/iceberg/test/bucket_util_test.cc
@@ -20,11 +20,16 @@
#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/bucket_util_internal.h"
#include "iceberg/util/decimal.h"
#include "iceberg/util/uuid.h"
@@ -108,6 +113,19 @@ 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. The guard rejects them before the
+ // data is touched, so pass a null pointer instead of building a std::span over
+ // more bytes than the object provides (which is undefined behavior) or
+ // allocating 2 GiB.
+ const auto oversized_length =
+ static_cast<size_t>(std::numeric_limits<int32_t>::max()) + 1;
+ ASSERT_THAT([&]() { internal::HashBytes(nullptr, oversized_length); },
+ ::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 2c0718f5..53a93d55 100644
--- a/src/iceberg/util/bucket_util.cc
+++ b/src/iceberg/util/bucket_util.cc
@@ -19,9 +19,12 @@
#include "iceberg/util/bucket_util.h"
+#include <limits>
#include <utility>
+#include "iceberg/exception.h"
#include "iceberg/expression/literal.h"
+#include "iceberg/util/bucket_util_internal.h"
#include "iceberg/util/endian.h"
#include "iceberg/util/murmurhash3_internal.h"
#include "iceberg/util/temporal_util.h"
@@ -110,11 +113,29 @@ int32_t HashLiteral<TypeId::kFixed>(const Literal& literal) {
} // namespace
int32_t BucketUtils::HashBytes(std::span<const uint8_t> bytes) {
+ return internal::HashBytes(bytes.data(), bytes.size());
+}
+
+namespace internal {
+
+int32_t HashBytes(const uint8_t* data, size_t len) {
+ // 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 `data`
+ // (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(
+ len <= static_cast<size_t>(std::numeric_limits<int32_t>::max()),
+ "BucketUtils::HashBytes: input length {} exceeds the maximum supported "
+ "length {}",
+ len, std::numeric_limits<int32_t>::max());
int32_t hash_value = 0;
- MurmurHash3_x86_32(bytes.data(), bytes.size(), 0, &hash_value);
+ MurmurHash3_x86_32(data, static_cast<int>(len), 0, &hash_value);
return hash_value;
}
+} // namespace internal
+
int32_t BucketUtils::HashLong(int64_t value) {
int32_t hash_value = 0;
value = ToLittleEndian(value);
diff --git a/src/iceberg/util/bucket_util.h b/src/iceberg/util/bucket_util.h
index 459f487b..3947d3fc 100644
--- a/src/iceberg/util/bucket_util.h
+++ b/src/iceberg/util/bucket_util.h
@@ -51,12 +51,19 @@ class ICEBERG_EXPORT BucketUtils {
/// \brief Hash a byte array using MurmurHash3 and return a 32-bit hash value.
/// \param bytes The input byte array to hash.
/// \return A 32-bit hash value.
+ /// \throws IcebergError if `bytes.size()` exceeds `INT32_MAX`. MurmurHash3 takes a
+ /// signed 32-bit length, and the bucket transform is only defined for byte
+ /// sequences up to that size, so longer inputs are rejected rather than
+ /// truncated. Java cannot produce such a value because its arrays are
+ /// int-indexed, so nothing iceberg-java could hash is rejected here.
static int32_t HashBytes(std::span<const uint8_t> bytes);
/// \brief Compute the bucket index for a given literal and number of buckets.
/// \param literal The input literal to hash.
/// \param num_buckets The number of buckets to hash into.
/// \return (murmur3_x86_32_hash(literal) & Integer.MAX_VALUE) % num_buckets
+ /// \throws IcebergError if the literal is a string, binary, or fixed value whose
+ /// byte length exceeds `INT32_MAX`; see HashBytes.
static Result<int32_t> BucketIndex(const Literal& literal, int32_t num_buckets);
};
diff --git a/src/iceberg/util/bucket_util_internal.h b/src/iceberg/util/bucket_util_internal.h
new file mode 100644
index 00000000..bda22dcc
--- /dev/null
+++ b/src/iceberg/util/bucket_util_internal.h
@@ -0,0 +1,45 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#pragma once
+
+#include <cstddef>
+#include <cstdint>
+
+#include "iceberg/iceberg_export.h"
+
+/// \file iceberg/util/bucket_util_internal.h
+/// \brief Internal entry points for the bucket transform.
+
+namespace iceberg::internal {
+
+/// \brief Hash `len` bytes starting at `data` using MurmurHash3.
+///
+/// The raw pointer and length are taken separately so that a length that does not
+/// fit the pointee object can be passed without forming an invalid `std::span`.
+/// `BucketUtils::HashBytes` forwards to this function.
+///
+/// \param data Pointer to the first byte to hash. It is only dereferenced once `len`
+/// is accepted, so it may be null when the call is expected to be rejected.
+/// \param len The number of bytes to hash.
+/// \return A 32-bit hash value.
+/// \throws IcebergError if `len` exceeds `INT32_MAX`; see `BucketUtils::HashBytes`.
+ICEBERG_EXPORT int32_t HashBytes(const uint8_t* data, size_t len);
+
+} // namespace iceberg::internal