From 0b01abbaa45e3794ed259e84adf442b8c680507c Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Tue, 1 Sep 2026 17:11:27 -0500 Subject: [PATCH 1/4] GH-50314: [C++][Parquet] Reject invalid DELTA_BINARY_PACKED headers InitHeader() sizes the bit-width buffer from the header's miniblock count without tying it to the page size, so a 10-byte page claiming 2^20 miniblocks allocates 1 MiB before failing. InitBlock() reads one bit-width byte per miniblock, so such a page can never decode. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 14 ++++++++++++++ cpp/src/parquet/encoding_test.cc | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index 47f2b46ebaac..486f3c009871 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1610,6 +1610,20 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { } total_values_remaining_ = total_value_count_; + // GH-50314: mini_blocks_per_block_ comes from the page header and sizes the + // allocation below, while InitBlock() reads one bit-width byte per miniblock. + // A count larger than the bytes left can never decode, so we reject it here + // instead of allowing it to drive a large allocation. A page holding a single + // value keeps that value in the header and never calls InitBlock(), so this + // check skips it. + if (total_value_count_ > 1 && + static_cast(mini_blocks_per_block_) > decoder_->bytes_left()) { + throw ParquetException( + "the number of miniblocks per block (" + + std::to_string(mini_blocks_per_block_) + + ") is larger than the number of bytes remaining in the page (" + + std::to_string(decoder_->bytes_left()) + ")"); + } if (delta_bit_widths_ == nullptr) { delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); } else { diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 74b422cb606f..acc1c9fabfc6 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -15,6 +15,7 @@ // specific language governing permissions and limitations // under the License. +#include #include #include @@ -31,6 +32,7 @@ #include "arrow/array/builder_dict.h" #include "arrow/array/concatenate.h" #include "arrow/compute/cast.h" +#include "arrow/memory_pool.h" #include "arrow/testing/gtest_util.h" #include "arrow/testing/random.h" #include "arrow/testing/util.h" @@ -1902,6 +1904,36 @@ TYPED_TEST(TestDeltaBitPackEncoding, BasicRoundTrip) { } } +TYPED_TEST(TestDeltaBitPackEncoding, SingleValueRoundTrip) { + ASSERT_NO_FATAL_FAILURE(this->Execute(1, 1)); +} + +TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { + using T = typename TypeParam::c_type; + + // Header: 2^25 values per block, 2^20 miniblocks, 2 values, and first value 0, + // followed by min delta 0 and no miniblock bit widths. + const std::vector encoded = {0x80, 0x80, 0x80, 0x10, 0x80, + 0x80, 0x40, 0x02, 0x00, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + std::vector decoded(2); + + EXPECT_THROW_THAT( + [&] { + decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->Decode(decoded.data(), static_cast(decoded.size())); + }, + ParquetException, + ::testing::Property( + &ParquetException::what, + ::testing::HasSubstr( + "the number of miniblocks per block (1048576) is larger than the " + "number of bytes remaining in the page (1)"))); + EXPECT_EQ(pool.bytes_allocated(), 0); +} + TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) { // GH-14923: depending on the number of encoded values, some of the miniblock // bitwidths are actually padding bytes that may take non-conformant values From 5e626bae04608f6cab22d11acdcdf208952e5345 Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Thu, 3 Sep 2026 14:31:53 -0400 Subject: [PATCH 2/4] GH-50314: [C++][Parquet] Avoid single-value miniblock allocation Single-value pages never initialize a block, so leave the bit-width buffer unallocated. Account for the required min-delta byte when validating block metadata, and use cumulative allocation counts in the regression tests. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 35 +++++++++++------------- cpp/src/parquet/encoding_test.cc | 46 +++++++++++++++++++++++++++++--- 2 files changed, 58 insertions(+), 23 deletions(-) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index 486f3c009871..e925899c271d 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1610,25 +1610,22 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { } total_values_remaining_ = total_value_count_; - // GH-50314: mini_blocks_per_block_ comes from the page header and sizes the - // allocation below, while InitBlock() reads one bit-width byte per miniblock. - // A count larger than the bytes left can never decode, so we reject it here - // instead of allowing it to drive a large allocation. A page holding a single - // value keeps that value in the header and never calls InitBlock(), so this - // check skips it. - if (total_value_count_ > 1 && - static_cast(mini_blocks_per_block_) > decoder_->bytes_left()) { - throw ParquetException( - "the number of miniblocks per block (" + - std::to_string(mini_blocks_per_block_) + - ") is larger than the number of bytes remaining in the page (" + - std::to_string(decoder_->bytes_left()) + ")"); - } - if (delta_bit_widths_ == nullptr) { - delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); - } else { - PARQUET_THROW_NOT_OK( - delta_bit_widths_->Resize(mini_blocks_per_block_, /*shrink_to_fit*/ false)); + if (total_value_count_ > 1) { + const int64_t bytes_left = decoder_->bytes_left(); + const int64_t bit_width_bytes_left = std::max(0, bytes_left - 1); + if (static_cast(mini_blocks_per_block_) > bit_width_bytes_left) { + throw ParquetException( + "the number of miniblocks per block (" + + std::to_string(mini_blocks_per_block_) + + ") is larger than the number of bytes available for miniblock bit widths (" + + std::to_string(bit_width_bytes_left) + ")"); + } + if (delta_bit_widths_ == nullptr) { + delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); + } else { + PARQUET_THROW_NOT_OK( + delta_bit_widths_->Resize(mini_blocks_per_block_, /*shrink_to_fit*/ false)); + } } first_block_initialized_ = false; values_remaining_current_mini_block_ = 0; diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index acc1c9fabfc6..002cc2385cd6 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -1904,8 +1904,21 @@ TYPED_TEST(TestDeltaBitPackEncoding, BasicRoundTrip) { } } -TYPED_TEST(TestDeltaBitPackEncoding, SingleValueRoundTrip) { - ASSERT_NO_FATAL_FAILURE(this->Execute(1, 1)); +TYPED_TEST(TestDeltaBitPackEncoding, SingleValueSkipsMiniblockAllocation) { + using T = typename TypeParam::c_type; + + // Header: 2^25 values per block, 2^20 miniblocks, 1 value, and first value 0. + const std::vector encoded = {0x80, 0x80, 0x80, 0x10, 0x80, + 0x80, 0x40, 0x01, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + T decoded = 1; + + decoder->SetData(1, encoded.data(), static_cast(encoded.size())); + ASSERT_EQ(decoder->Decode(&decoded, 1), 1); + EXPECT_EQ(decoded, 0); + EXPECT_EQ(pool.total_bytes_allocated(), 0); } TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { @@ -1930,8 +1943,33 @@ TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { &ParquetException::what, ::testing::HasSubstr( "the number of miniblocks per block (1048576) is larger than the " - "number of bytes remaining in the page (1)"))); - EXPECT_EQ(pool.bytes_allocated(), 0); + "number of bytes available for miniblock bit widths (0)"))); + EXPECT_EQ(pool.total_bytes_allocated(), 0); +} + +TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsWithoutMinDelta) { + using T = typename TypeParam::c_type; + + // Header: 128 values per block, 1 miniblock, 2 values, and first value 0, + // followed by only one byte for the min delta and miniblock bit width. + const std::vector encoded = {0x80, 0x01, 0x01, 0x02, 0x00, 0x00}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + std::vector decoded(2); + + EXPECT_THROW_THAT( + [&] { + decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->Decode(decoded.data(), static_cast(decoded.size())); + }, + ParquetException, + ::testing::Property( + &ParquetException::what, + ::testing::HasSubstr( + "the number of miniblocks per block (1) is larger than the number " + "of bytes available for miniblock bit widths (0)"))); + EXPECT_EQ(pool.total_bytes_allocated(), 0); } TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) { From fca97c622d5d91e50c44ea8d798018699d742aec Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Wed, 23 Sep 2026 23:29:44 -0700 Subject: [PATCH 3/4] GH-50314: [C++][Parquet] Size miniblock buffer after min delta InitHeader subtracted one byte for min delta, so a two-byte min delta with no bit widths still allocated the aligned scratch buffer and then failed with Decode bit-width EOF. Consume min delta first, then reject when remaining bytes cannot hold the declared miniblock widths. Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 15 ++++++++++----- cpp/src/parquet/encoding_test.cc | 25 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index e925899c271d..f36f2777e902 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1611,14 +1611,17 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { total_values_remaining_ = total_value_count_; if (total_value_count_ > 1) { + // Read min delta before allocating because it can consume more than one byte. + if (!decoder_->GetZigZagVlqInt(&min_delta_)) { + ParquetException::EofException("InitBlock EOF"); + } const int64_t bytes_left = decoder_->bytes_left(); - const int64_t bit_width_bytes_left = std::max(0, bytes_left - 1); - if (static_cast(mini_blocks_per_block_) > bit_width_bytes_left) { + if (static_cast(mini_blocks_per_block_) > bytes_left) { throw ParquetException( "the number of miniblocks per block (" + std::to_string(mini_blocks_per_block_) + ") is larger than the number of bytes available for miniblock bit widths (" + - std::to_string(bit_width_bytes_left) + ")"); + std::to_string(bytes_left) + ")"); } if (delta_bit_widths_ == nullptr) { delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); @@ -1634,8 +1637,10 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { void InitBlock() { DCHECK_GT(total_values_remaining_, 0) << "InitBlock called at EOF"; - if (!decoder_->GetZigZagVlqInt(&min_delta_)) - ParquetException::EofException("InitBlock EOF"); + if (first_block_initialized_) { + if (!decoder_->GetZigZagVlqInt(&min_delta_)) + ParquetException::EofException("InitBlock EOF"); + } // read the bitwidth of each miniblock uint8_t* bit_width_data = delta_bit_widths_->mutable_data(); diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 002cc2385cd6..6247d92667c3 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -1972,6 +1972,31 @@ TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsWithoutMinDelta) { EXPECT_EQ(pool.total_bytes_allocated(), 0); } +TYPED_TEST(TestDeltaBitPackEncoding, RejectsTruncatedMultiByteMinDeltaWithoutAllocating) { + using T = typename TypeParam::c_type; + + // Header: 128 values per block, 1 miniblock, 2 values, and first value 0, + // followed by a two-byte min delta and no miniblock bit-width byte. + const std::vector encoded = {0x80, 0x01, 0x01, 0x02, 0x00, 0x80, 0x01}; + ::arrow::ProxyMemoryPool pool(default_memory_pool()); + auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, + this->descr_.get(), &pool); + std::vector decoded(2); + + EXPECT_THROW_THAT( + [&] { + decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->Decode(decoded.data(), static_cast(decoded.size())); + }, + ParquetException, + ::testing::Property( + &ParquetException::what, + ::testing::HasSubstr( + "the number of miniblocks per block (1) is larger than the number " + "of bytes available for miniblock bit widths (0)"))); + EXPECT_EQ(pool.total_bytes_allocated(), 0); +} + TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) { // GH-14923: depending on the number of encoded values, some of the miniblock // bitwidths are actually padding bytes that may take non-conformant values From 025e74816fb6725c8f46a87a60aedfff5d14527a Mon Sep 17 00:00:00 2001 From: 1fanwang <1fannnw@gmail.com> Date: Tue, 6 Oct 2026 11:19:36 -0700 Subject: [PATCH 4/4] GH-50314: Validate DELTA_BINARY_PACKED bit-width bounds Signed-off-by: 1fanwang <1fannnw@gmail.com> --- cpp/src/parquet/decoder.cc | 25 +++++++++----------- cpp/src/parquet/encoding_test.cc | 39 ++++++++++++++------------------ 2 files changed, 28 insertions(+), 36 deletions(-) diff --git a/cpp/src/parquet/decoder.cc b/cpp/src/parquet/decoder.cc index f36f2777e902..b64349579fc7 100644 --- a/cpp/src/parquet/decoder.cc +++ b/cpp/src/parquet/decoder.cc @@ -1611,17 +1611,16 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { total_values_remaining_ = total_value_count_; if (total_value_count_ > 1) { - // Read min delta before allocating because it can consume more than one byte. - if (!decoder_->GetZigZagVlqInt(&min_delta_)) { - ParquetException::EofException("InitBlock EOF"); - } + const int64_t required_blocks = ::arrow::bit_util::CeilDiv( + static_cast(total_value_count_) - 1, values_per_block_); + const int64_t required_bit_widths = + required_blocks * static_cast(mini_blocks_per_block_); const int64_t bytes_left = decoder_->bytes_left(); - if (static_cast(mini_blocks_per_block_) > bytes_left) { - throw ParquetException( - "the number of miniblocks per block (" + - std::to_string(mini_blocks_per_block_) + - ") is larger than the number of bytes available for miniblock bit widths (" + - std::to_string(bytes_left) + ")"); + if (required_bit_widths > bytes_left) { + throw ParquetException("the required number of miniblock bit widths (" + + std::to_string(required_bit_widths) + + ") exceeds the number of bytes remaining in the data (" + + std::to_string(bytes_left) + ")"); } if (delta_bit_widths_ == nullptr) { delta_bit_widths_ = AllocateBuffer(pool_, mini_blocks_per_block_); @@ -1637,10 +1636,8 @@ class DeltaBitPackDecoder : public TypedDecoderImpl { void InitBlock() { DCHECK_GT(total_values_remaining_, 0) << "InitBlock called at EOF"; - if (first_block_initialized_) { - if (!decoder_->GetZigZagVlqInt(&min_delta_)) - ParquetException::EofException("InitBlock EOF"); - } + if (!decoder_->GetZigZagVlqInt(&min_delta_)) + ParquetException::EofException("InitBlock EOF"); // read the bitwidth of each miniblock uint8_t* bit_width_data = delta_bit_widths_->mutable_data(); diff --git a/cpp/src/parquet/encoding_test.cc b/cpp/src/parquet/encoding_test.cc index 6247d92667c3..82493a2d0077 100644 --- a/cpp/src/parquet/encoding_test.cc +++ b/cpp/src/parquet/encoding_test.cc @@ -1941,46 +1941,45 @@ TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsLargerThanInput) { ParquetException, ::testing::Property( &ParquetException::what, - ::testing::HasSubstr( - "the number of miniblocks per block (1048576) is larger than the " - "number of bytes available for miniblock bit widths (0)"))); + ::testing::HasSubstr("the required number of miniblock bit widths " + "(1048576) exceeds the number of bytes remaining " + "in the data (1)"))); EXPECT_EQ(pool.total_bytes_allocated(), 0); } -TYPED_TEST(TestDeltaBitPackEncoding, RejectsMiniblockWidthsWithoutMinDelta) { +TYPED_TEST(TestDeltaBitPackEncoding, RejectsInsufficientBitWidthsAcrossBlocks) { using T = typename TypeParam::c_type; - // Header: 128 values per block, 1 miniblock, 2 values, and first value 0, - // followed by only one byte for the min delta and miniblock bit width. - const std::vector encoded = {0x80, 0x01, 0x01, 0x02, 0x00, 0x00}; + // Header: 128 values per block, 1 miniblock, 130 values, and first value 0. + const std::vector encoded = {0x80, 0x01, 0x01, 0x82, 0x01, 0x00, 0x00}; ::arrow::ProxyMemoryPool pool(default_memory_pool()); auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, this->descr_.get(), &pool); - std::vector decoded(2); + std::vector decoded(130); EXPECT_THROW_THAT( [&] { - decoder->SetData(2, encoded.data(), static_cast(encoded.size())); + decoder->SetData(static_cast(decoded.size()), encoded.data(), + static_cast(encoded.size())); decoder->Decode(decoded.data(), static_cast(decoded.size())); }, ParquetException, ::testing::Property( &ParquetException::what, - ::testing::HasSubstr( - "the number of miniblocks per block (1) is larger than the number " - "of bytes available for miniblock bit widths (0)"))); + ::testing::HasSubstr("the required number of miniblock bit widths (2) " + "exceeds the number of bytes remaining in the " + "data (1)"))); EXPECT_EQ(pool.total_bytes_allocated(), 0); } -TYPED_TEST(TestDeltaBitPackEncoding, RejectsTruncatedMultiByteMinDeltaWithoutAllocating) { +TYPED_TEST(TestDeltaBitPackEncoding, RejectsMissingBitWidthAfterMultiByteMinDelta) { using T = typename TypeParam::c_type; // Header: 128 values per block, 1 miniblock, 2 values, and first value 0, // followed by a two-byte min delta and no miniblock bit-width byte. const std::vector encoded = {0x80, 0x01, 0x01, 0x02, 0x00, 0x80, 0x01}; - ::arrow::ProxyMemoryPool pool(default_memory_pool()); - auto decoder = MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, - this->descr_.get(), &pool); + auto decoder = + MakeTypedDecoder(Encoding::DELTA_BINARY_PACKED, this->descr_.get()); std::vector decoded(2); EXPECT_THROW_THAT( @@ -1989,12 +1988,8 @@ TYPED_TEST(TestDeltaBitPackEncoding, RejectsTruncatedMultiByteMinDeltaWithoutAll decoder->Decode(decoded.data(), static_cast(decoded.size())); }, ParquetException, - ::testing::Property( - &ParquetException::what, - ::testing::HasSubstr( - "the number of miniblocks per block (1) is larger than the number " - "of bytes available for miniblock bit widths (0)"))); - EXPECT_EQ(pool.total_bytes_allocated(), 0); + ::testing::Property(&ParquetException::what, + ::testing::HasSubstr("Decode bit-width EOF"))); } TYPED_TEST(TestDeltaBitPackEncoding, NonZeroPaddedMiniblockBitWidth) {