nlebovits opened a new issue, #12100:
URL: https://github.com/apache/arrow-rs/issues/12100
### Describe the bug
`ThriftReadInputProtocol::read_bytes_owned` (parquet/src/parquet_thrift.rs)
reads a varint length from the file and calls `Vec::with_capacity(len)` before
reading any data. The length is not compared with the remaining input. A page
header whose statistics declare a very large `binary` value aborts the process
with an allocation failure instead of returning an error. This is reachable
from the public API when page statistics are enabled
(`ReaderPropertiesBuilder::set_read_page_statistics(true)`), for example
through `SerializedPageReader`. A smaller length (1 GB) allocates 1 GB for a
file of a few hundred bytes.
Found while fuzzing a downstream GeoParquet reader. This is the `Read`-based
counterpart of the checks added for `read_thrift_vec` in #10979.
### To Reproduce
`Cargo.toml`:
```toml
[dependencies]
parquet = { version = "60.1.0", default-features = false }
bytes = "1"
```
`main.rs` (the argument is the declared length):
```rust
use bytes::Bytes;
use parquet::basic::Type;
use parquet::column::writer::ColumnWriter;
use parquet::file::properties::WriterProperties;
use parquet::file::properties::ReaderProperties;
use parquet::file::reader::{FileReader, SerializedFileReader};
use parquet::file::serialized_reader::ReadOptionsBuilder;
use parquet::file::writer::SerializedFileWriter;
use parquet::schema::types::Type as SchemaType;
use std::sync::Arc;
use std::time::Instant;
fn valid_file() -> Vec<u8> {
let schema = SchemaType::group_type_builder("s")
.with_fields(vec![Arc::new(
SchemaType::primitive_type_builder("a",
Type::INT32).with_repetition(parquet::basic::Repetition::REQUIRED).build().unwrap(),
)])
.build()
.unwrap();
let mut out = Vec::new();
let props = Arc::new(WriterProperties::builder().build());
let mut w = SerializedFileWriter::new(&mut out, Arc::new(schema),
props).unwrap();
let mut rg = w.next_row_group().unwrap();
let mut col = rg.next_column().unwrap().unwrap();
if let ColumnWriter::Int32ColumnWriter(c) = col.untyped() {
c.write_batch(&[1, 2, 3], None, None).unwrap();
}
col.close().unwrap();
rg.close().unwrap();
w.close().unwrap();
out
}
fn main() {
let n: u64 = std::env::args().nth(1).unwrap().parse().unwrap();
let v = valid_file();
// footer = thrift metadata + 4-byte length + "PAR1"
let meta_len = u32::from_le_bytes(v[v.len() - 8..v.len() -
4].try_into().unwrap()) as usize;
let footer = &v[v.len() - 8 - meta_len..];
// Page header (type=0, sizes=100) with data_page_header { num_values,
encoding,
// def/rep level encodings, statistics { field 1 (binary) with declared
length `n` } }
let mut f = b"PAR1".to_vec();
f.extend_from_slice(&[0x15, 0x00, 0x15, 0xc8, 0x01, 0x15, 0xc8, 0x01]);
f.extend_from_slice(&[0x2c, 0x15, 0x06, 0x15, 0x00, 0x15, 0x00, 0x15,
0x00, 0x1c, 0x18]);
let mut n_ = n;
while n_ >= 0x80 {
f.push((n_ & 0x7f) as u8 | 0x80);
n_ >>= 7;
}
f.push(n_ as u8);
f.extend_from_slice(footer);
let t = Instant::now();
let r = SerializedFileReader::new_with_options(
Bytes::from(f),
ReadOptionsBuilder::new()
.with_reader_properties(ReaderProperties::builder().set_read_page_statistics(true).build())
.build(),
)
.unwrap();
let rg = r.get_row_group(0).unwrap();
let mut pr = rg.get_column_page_reader(0).unwrap();
println!("n={n}: {:?}", pr.get_next_page().map(|p| p.map(|p|
p.page_type())));
println!("elapsed {:?}", t.elapsed());
}
```
Output on parquet 60.1.0 (`ulimit -v 2000000; timeout 60 ./repro-3
1099511627776`):
```
memory allocation of 1099511627776 bytes failed
timeout: the monitored command dumped core (exit 134)
```
With `1000` or `1000000000` the call returns `Err(External(... UnexpectedEof
...))`, after reserving that much memory. Same behaviour on current main
(bb47f80).
### Expected behavior
`Err(ParquetError)`, without a large up-front allocation.
### Additional context
Suggested fix: do not trust `len` for the reservation. Either read through
`Read::take(len).read_to_end(&mut v)` into an empty `Vec` (it grows as data
arrives), or cap the reserve (for example `Vec::with_capacity(len.min(8192))`),
and return `Eof` if fewer than `len` bytes were read. Environment: parquet
60.1.0 and main bb47f80, rustc 1.99.1, Linux aarch64.
--
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]
