apache / apache/arrow-rs

parquet: panic-on-decode DoS from unchecked Thrift list size in read_thrift_vec (and broader thrift parser hardening)

Open
#9,874 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
3.6k
Forks
1.3k
Avg merge
2d 18h
Merged PRs (30d)
169

Description

## Describe the bug

`parquet::parquet_thrift::read_thrift_vec` reads a Thrift compact-protocol list header and then calls `Vec::with_capacity(list_ident.size as usize)`, where `list_ident.size` is a 32-bit varint pulled directly from attacker-controlled bytes. With a malformed input, the value can be close to `i32::MAX`; after the per-element-size multiplication this becomes a multi-GB to multi-hundred-GB allocation request, which panics in `alloc::raw_vec::handle_error` with `capacity overflow` (or OOM-kills the process on smaller-but-still-huge values) before any element is decoded.

Every public sync metadata entry point funnels through this function:

- `ParquetMetaDataReader::parse_and_finish`
- `ParquetMetaDataReader::decode_metadata`
- `SerializedFileReader::new`
- `ParquetRecordBatchReaderBuilder::try_new`
- the `async_reader` family (re-exports `decode_metadata` after prefetching footer bytes)

So any downstream code that hands attacker-controlled bytes to a parquet reader gets a panic-on-decode DoS.

Per `SECURITY.md` this is a **bug, not a vulnerability** — there is no information disclosure or RCE path, only availability — but it is reachable from every metadata entry point, so it is worth tracking and closing properly.

## To Reproduce

10 bytes:

```
0x28 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0xfc 0x51
```

```rust
let bytes: &[u8] = &[0x28, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0xfc, 0x51];
let _ = parquet::file::metadata::ParquetMetaDataReader::decode_metadata(bytes);
```

Output (with `RUST_BACKTRACE=1`):

```
panicked at library/alloc/src/raw_vec/mod.rs:28:5: capacity overflow
3: alloc::raw_vec::handle_error
4: alloc::raw_vec::with_capacity_in
7: alloc::vec::Vec::with_capacity::
8: parquet::parquet_thrift::read_thrift_vec (parquet_thrift.rs:690)
9: parquet::file::metadata::thrift::parquet_metadata_from_bytes
(thrift/mod.rs:790)
10: parquet::file::metadata::parser::decode_metadata (parser.rs:233)
13: ParquetMetaDataReader::decode_footer_metadata (reader.rs:783)
14: ParquetMetaDataReader::parse_metadata (reader.rs:585)
17: ParquetMetaDataReader::parse_and_finish (reader.rs:230)
```

A 45-byte reproducer with valid `PAR1` magic drives the same panic through `ParquetRecordBatchReaderBuilder::try_new`.

## Expected behavior

Malformed Parquet input should return a `ParquetError` that callers can handle, never panic.

## Found via

`cargo-fuzz` libFuzzer harness wrapping `ParquetMetaDataReader::parse_and_finish` and `ParquetRecordBatchReaderBuilder::try_new` over `bytes::Bytes`. ~95 unique crashing inputs (different VLQ sizes, different element types — `SchemaElement`, `RowGroup`, `ColumnChunk` — different offsets in the metadata graph) all converged on this single root cause within ~3 minutes of single-thread fuzzing. **One bug, many surface symptoms.**

## Scope

This issue tracks two related strands:

1. **Immediate fix** — bound the `Vec::with_capacity` in `read_thrift_vec` by remaining input bytes, since every Thrift element costs at least one wire byte. Also reject negative declared sizes explicitly. Proposed in #9868.
2. **Broader thrift parser hardening** — per @etseidl on #9868: \"I think we could make a few more improvements to the thrift parser here.\" This issue is the umbrella for those follow-ups (e.g. reviewing every `with_capacity`/`reserve` in `parquet_thrift` against attacker-controlled sizes, audit of nested-list / map-key-or-value paths, any other decoders that could allocate before reading).

Happy to do further fuzzing passes against specific suspected hotspots if maintainers point at where to look.

## Environment

- crate: `parquet`
- master at the time of report: HEAD around `fd86c75` (see #9868 merge-base)
- rustc: stable

## Related

- PR #9868 — proposed fix for strand 1.

Contributor guide

Open the contributing guide

Research direction

Start with parquet_thrift.rs and read_thrift_vec, then trace the listed metadata entry points and review the related PR #9868. Run the 10-byte reproducer and relevant parquet tests; done means malformed metadata returns ParquetError without panic, with the broader attacker-controlled allocation paths reviewed.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.