elshanhsynv opened a new issue, #12129:
URL: https://github.com/apache/arrow-rs/issues/12129
### Describe the bug
Reading a primitive (e.g. `INT32`) column whose data page is
`RLE_DICTIONARY`-encoded, when no dictionary page was read for that column
chunk, panics instead of returning an error:
```
thread 'main' panicked at parquet-60.1.0/src/column/reader/decoder.rs:374:18:
Decoder for dict should have been set
```
`ColumnValueDecoderImpl::set_data` calls `.expect("Decoder for dict should
have been set")` on the dictionary decoder slot. The byte-array readers already
handle this case and return `missing dictionary page for column`
(`arrow/array_reader/byte_array.rs`), so this looks like the same check missing
from the generic decoder.
A malformed or truncated file can trigger this, so any application reading
untrusted Parquet files can be crashed by it. We found it by fuzzing a Parquet
viewer with libFuzzer.
### To Reproduce
Write a dictionary-encoded `Int32` column, then change the dictionary page
header's type from `DICTIONARY_PAGE` (2) to `INDEX_PAGE` (1). The reader skips
the page as an index page, and the following data page has no dictionary.
```toml
[dependencies]
parquet = { version = "60.1.0", features = ["arrow"] }
arrow-array = "60.1.0"
bytes = "1"
```
```rust
use std::sync::Arc;
use arrow_array::{Int32Array, RecordBatch};
use bytes::Bytes;
use parquet::{
arrow::{ArrowWriter, arrow_reader::ParquetRecordBatchReaderBuilder},
file::reader::{FileReader, SerializedFileReader},
};
fn main() {
let column = Int32Array::from(vec![7, 8, 7, 8]);
let batch = RecordBatch::try_from_iter([("i", Arc::new(column) as
_)]).unwrap();
let mut bytes = Vec::new();
let mut writer = ArrowWriter::try_new(&mut bytes, batch.schema(),
None).unwrap();
writer.write(&batch).unwrap();
writer.close().unwrap();
let reader =
SerializedFileReader::new(Bytes::from(bytes.clone())).unwrap();
let offset = reader
.metadata()
.row_group(0)
.column(0)
.dictionary_page_offset()
.unwrap() as usize;
// Compact-protocol PageHeader starts with field 1 (type, i32): 0x15,
then zigzag(2) = 0x04.
assert_eq!(&bytes[offset..offset + 2], &[0x15, 0x04], "dictionary page
header");
bytes[offset + 1] = 0x02; // zigzag(1) = INDEX_PAGE
let reader = ParquetRecordBatchReaderBuilder::try_new(Bytes::from(bytes))
.unwrap()
.build()
.unwrap();
for batch in reader {
println!("{:?}", batch.map(|b| b.num_rows())); // expected:
Err(...), actual: panic
}
}
```
The same program with a `StringArray` column returns
`Err(ParquetError("Parquet error: missing dictionary page for column"))`, which
is the expected behavior.
### Expected behavior
`Err(ParquetError::General("missing dictionary page for column"))` (or
similar), not a panic.
### Additional context
- Reproduces on 60.1.0 and 59.2.0 (`decoder.rs:213` there), Windows x86_64,
rustc 1.97.1.
- Found with cargo-fuzz; the fuzzer's original input was a mutated file, and
the program above is a minimal deterministic equivalent.
--
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]
