hhhizzz opened a new issue, #11516:
URL: https://github.com/apache/arrow-rs/issues/11516
### Describe the bug
`RleDecoder::reload` computes the length of a bit-packed run from its header
without checking for overflow (`parquet/src/encodings/rle.rs`):
```rust
self.bit_packed_left = ((indicator_value >> 1) * BIT_PACK_GROUP_SIZE as i64)
as u32;
```
`indicator_value` is a varint of up to 10 bytes, so a corrupt header can
decode to any `i64`. With overflow checks enabled (e.g. debug builds) the
multiplication panics:
```
thread 'main' panicked at parquet/src/encodings/rle.rs:676:40:
attempt to multiply with overflow
```
Without overflow checks the length wraps and decoding continues with a bogus
run length, so the read fails later with an unrelated error, or panics
elsewhere depending on the bytes that follow. The RLE branch truncates the run
length to `u32` the same way, and `PackedDecoder::next_rle_block`, which parses
run headers for the definition level fast path, has the same unchecked `len *
8`.
### To Reproduce
With `parquet = "60.0.0"`, `arrow-array = "60.0.0"` and `bytes = "1"`:
```rust
use std::sync::Arc;
use arrow_array::{ArrayRef, Int32Array, RecordBatch};
use bytes::Bytes;
use parquet::arrow::ArrowWriter;
use parquet::arrow::arrow_reader::ParquetRecordBatchReaderBuilder;
fn main() {
// 128 dictionary indices without repeats are written as a single
bit-packed
// run: bit width byte 0x03, then run header 0x21 (16 groups of 8 values)
let values: ArrayRef =
Arc::new(Int32Array::from_iter_values((0..128).map(|i| i % 8)));
let batch = RecordBatch::try_from_iter_with_nullable([("v", values,
false)]).unwrap();
let mut file = Vec::new();
let mut writer = ArrowWriter::try_new(&mut file, batch.schema(),
None).unwrap();
writer.write(&batch).unwrap();
writer.close().unwrap();
// Overwrite the run header with a 10 byte varint for
0xa4ff_ffff_ffff_ffff,
// a bit-packed run whose length in values overflows i64
let header = file.windows(2).position(|w| w == [0x03, 0x21]).unwrap() +
1;
file[header..header + 10]
.copy_from_slice(&[0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xa4, 0x01]);
let mut reader =
ParquetRecordBatchReaderBuilder::try_new(Bytes::from(file))
.unwrap()
.build()
.unwrap();
println!("{:?}", reader.next());
}
```
In a debug build this panics at `rle.rs:632` (`rle.rs:676` on `main`). In a
release build it prints:
```
Some(Err(ParquetError("Parquet error: insufficient values read from column -
expected: 128, got: 104")))
```
### Expected behavior
`next()` returns an error that points at the invalid run header. The
[Parquet
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]`, and Arrow C++ rejects
longer runs.
### Additional context
Found by mutation testing Parquet files in a downstream project. Reproduced
with 60.0.0 and `main` (0706d317a).
This report was drafted with the help of AI Agent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
