You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by gi...@apache.org on 2026/10/20 04:36:58 UTC
(arrow-rs) branch gh-readonly-queue/main/pr-11517-1357f25f26ca67634e992bdb35220b4ee59c780f created (now 6f3d922539)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-11517-1357f25f26ca67634e992bdb35220b4ee59c780f
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
at 6f3d922539 fix(parquet): return an error for RLE run lengths above the spec limit (#11517)
This branch includes the following new commits:
new 6f3d922539 fix(parquet): return an error for RLE run lengths above the spec limit (#11517)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(arrow-rs) 01/01: fix(parquet): return an error for RLE run lengths above the spec limit (#11517)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-11517-1357f25f26ca67634e992bdb35220b4ee59c780f
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
commit 6f3d92253941ac2d72572c44ab174656b5f5bc13
Author: Huang Qiwei <qi...@qq.com>
AuthorDate: Tue Oct 20 04:36:33 2026 +0000
fix(parquet): return an error for RLE run lengths above the spec limit (#11517)
# Which issue does this PR close?
- Closes #11516.
# Rationale for this change
A corrupt RLE / bit-packed hybrid run header can decode to any 64-bit
value. `RleDecoder::reload` multiplied the bit-packed group count by 8
in `i64` without a check, so reading such a page panics with `attempt to
multiply with overflow` when overflow checks are enabled (e.g. debug
builds). Otherwise the length wraps, decoding continues with a bogus run
length, and the read fails later with an unrelated error or panics
elsewhere. `PackedDecoder`, the definition level fast path, parses run
headers itself and has the same unchecked multiplication.
# What changes are included in this PR?
- Add `run_len`, which returns the number of values in a run, or an
error if it is above 2^31 - 1, the maximum run length allowed by the
[Parquet
spec](https://github.com/apache/parquet-format/blob/master/Encodings.md#run-length-encoding--bit-packing-hybrid-rle--3).
Arrow C++ enforces the same limit.
- Use it in `RleDecoder::reload` and `PackedDecoder::next_rle_block`.
# Are these changes tested?
Yes. `test_run_len_limit` checks that the longest runs allowed by the
spec are accepted and that longer ones are rejected, including headers
whose length overflows `i64` and headers that are negative as `i64`.
`test_packed_decoder_invalid_run_len` covers `PackedDecoder`. Both tests
fail without this change.
# Are there any significant user-facing changes?
No.
---
This PR was written with the help of Claude Code and Codex: the fix, the
tests and this description were AI-generated.
I have reviewed the change, and it was verified locally, including
against the corrupt file that first triggered the panic.
---------
Co-authored-by: Claude Opus 5.5 <no...@anthropic.com>
Co-authored-by: Ed Seidl <et...@users.noreply.github.com>
Co-authored-by: Hippolyte Barraud <hi...@gmail.com>
---
.../src/arrow/record_reader/definition_levels.rs | 23 +++++++-
parquet/src/encodings/rle.rs | 65 ++++++++++++++++++++--
2 files changed, 81 insertions(+), 7 deletions(-)
diff --git a/parquet/src/arrow/record_reader/definition_levels.rs b/parquet/src/arrow/record_reader/definition_levels.rs
index e937236ac1..46def4e798 100644
--- a/parquet/src/arrow/record_reader/definition_levels.rs
+++ b/parquet/src/arrow/record_reader/definition_levels.rs
@@ -26,6 +26,7 @@ use crate::basic::Encoding;
use crate::column::reader::decoder::{
ColumnLevelDecoder, DefinitionLevelDecoder, DefinitionLevelDecoderImpl,
};
+use crate::encodings::rle::run_len;
use crate::errors::{ParquetError, Result};
use crate::schema::types::ColumnDescPtr;
@@ -330,12 +331,12 @@ struct PackedDecoder {
impl PackedDecoder {
fn next_rle_block(&mut self) -> Result<()> {
let indicator_value = self.decode_header()?;
+ let num_values = run_len(indicator_value as u64)? as usize;
if indicator_value & 1 == 1 {
- let len = (indicator_value >> 1) as usize;
- self.packed_count = len * 8;
+ self.packed_count = num_values;
self.packed_offset = 0;
} else {
- self.rle_left = (indicator_value >> 1) as usize;
+ self.rle_left = num_values;
let byte = *self.data.as_ref().get(self.data_offset).ok_or_else(|| {
ParquetError::EOF(
"unexpected end of file whilst decoding definition levels rle value".into(),
@@ -499,6 +500,7 @@ mod tests {
use super::*;
use crate::encodings::rle::RleEncoder;
+ use crate::util::bit_util::BitWriter;
use rand::{RngExt, rng};
#[test]
@@ -683,4 +685,19 @@ mod tests {
let result = decoder.try_consume_all_valid(5).unwrap();
assert_eq!(result, None);
}
+
+ #[test]
+ fn test_packed_decoder_invalid_run_len() {
+ let mut writer = BitWriter::new(16);
+ writer.put_vlq_int(u64::MAX);
+ let mut decoder = PackedDecoder::new();
+ decoder.set_data(Encoding::RLE, writer.consume().into());
+
+ let mut decoded = BooleanBufferBuilder::new(8);
+ let err = decoder.read(&mut decoded, 8).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ "Parquet error: Invalid or corrupted RLE run length 18446744073709551615. Max allowed is 2147483647"
+ );
+ }
}
diff --git a/parquet/src/encodings/rle.rs b/parquet/src/encodings/rle.rs
index 786197bf5b..d45b999fc0 100644
--- a/parquet/src/encodings/rle.rs
+++ b/parquet/src/encodings/rle.rs
@@ -56,6 +56,30 @@ const BIT_PACK_GROUP_SIZE: usize = 8;
/// Maximum groups of `BIT_PACK_GROUP_SIZE` values per bit-packed run. Current value is 64.
const MAX_GROUPS_PER_BIT_PACKED_RUN: usize = 1 << 6;
+/// Maximum number of values in a single run. The
+/// [format spec](https://github.com/apache/parquet-format/blob/master/Encodings.md#run-length-encoding--bit-packing-hybrid-rle--3)
+/// requires run lengths to be in the range `[1, 2^31 - 1]`.
+const MAX_RUN_LEN: u64 = i32::MAX as u64;
+
+/// Returns the number of values in the run with the given header, or an error if it
+/// exceeds [`MAX_RUN_LEN`].
+#[inline]
+pub(crate) fn run_len(indicator_value: u64) -> Result<u32> {
+ let len = if indicator_value & 1 == 1 {
+ // A bit-packed header stores the number of groups of `BIT_PACK_GROUP_SIZE` values.
+ // Saturate so that a corrupt header which overflows is rejected below.
+ (indicator_value >> 1).saturating_mul(BIT_PACK_GROUP_SIZE as u64)
+ } else {
+ indicator_value >> 1
+ };
+ if len > MAX_RUN_LEN {
+ return Err(general_err!(
+ "Invalid or corrupted RLE run length {len}. Max allowed is {MAX_RUN_LEN}"
+ ));
+ }
+ Ok(len as u32)
+}
+
/// A RLE/Bit-Packing hybrid encoder.
// TODO: tracking memory usage
pub struct RleEncoder {
@@ -712,10 +736,11 @@ impl RleDecoder {
if indicator_value == 0 {
return Ok(false);
}
+ let num_values = run_len(indicator_value as u64)?;
if indicator_value & 1 == 1 {
- self.bit_packed_left = ((indicator_value >> 1) * BIT_PACK_GROUP_SIZE as i64) as u32;
+ self.bit_packed_left = num_values;
} else {
- self.rle_left = (indicator_value >> 1) as u32;
+ self.rle_left = num_values;
let value_width = bit_util::ceil(self.bit_width as usize, u8::BITS as usize);
self.current_value = bit_reader.get_aligned::<u64>(value_width);
self.current_value.ok_or_else(|| {
@@ -857,14 +882,14 @@ mod tests {
#[test]
fn test_skip_and_count_long_run() {
let mut writer = BitWriter::new(8);
- writer.put_vlq_int(u64::from(u32::MAX) * 2);
+ writer.put_vlq_int(MAX_RUN_LEN << 1);
writer.put_aligned(1_u8, 1);
let mut decoder = RleDecoder::new(1);
decoder.set_data(writer.consume().into()).unwrap();
// Must not expand this run or allocate scratch space proportional to it.
assert_eq!(
decoder.skip_and_count(usize::MAX, 1_u64).unwrap(),
- (u32::MAX as usize, u32::MAX as usize)
+ (MAX_RUN_LEN as usize, MAX_RUN_LEN as usize)
);
assert_eq!(decoder.skip_and_count(1, 1_u64).unwrap(), (0, 0));
}
@@ -1371,6 +1396,38 @@ mod tests {
assert_eq!(vec![23; num_values], decoded);
}
+ #[test]
+ fn test_run_len_limit() {
+ let set_data = |indicator_value: u64| {
+ let mut writer = BitWriter::new(16);
+ writer.put_vlq_int(indicator_value);
+ writer.put_aligned(1_u8, 1); // repeated value of an RLE run
+ RleDecoder::new(1).set_data(writer.consume().into())
+ };
+ let max_groups = MAX_RUN_LEN / BIT_PACK_GROUP_SIZE as u64;
+
+ // The longest runs allowed by the spec
+ set_data(MAX_RUN_LEN << 1).unwrap();
+ set_data((max_groups << 1) | 1).unwrap();
+
+ for (indicator_value, expected_len) in [
+ ((MAX_RUN_LEN + 1) << 1, "2147483648"),
+ (((max_groups + 1) << 1) | 1, "2147483648"),
+ // Overflows `i64` when converted to a number of values
+ ((1 << 61) | 1, "9223372036854775808"),
+ // Negative when decoded as `i64`
+ (0xa4ff_ffff_ffff_ffff, "18446744073709551615"),
+ ] {
+ let err = set_data(indicator_value).unwrap_err();
+ assert_eq!(
+ err.to_string(),
+ format!(
+ "Parquet error: Invalid or corrupted RLE run length {expected_len}. Max allowed is 2147483647"
+ )
+ );
+ }
+ }
+
#[test]
fn test_rle_specific_roundtrip() {
let bit_width = 1;