sivaadityacoder opened a new pull request, #49954:
URL: https://github.com/apache/arrow/pull/49954
# GH-XXXXX: [C++] Fix OOM vulnerability in Parquet Delta decoders
## Rationale for this change
This PR addresses an uncontrolled memory allocation vulnerability in the
Parquet `DeltaByteArrayDecoder`, `DeltaLengthByteArrayDecoder`, and
`DeltaBitPackDecoder`.
Currently, these decoders trust the `num_values` (and implicitly the
`total_value_count_`) provided by the Parquet data page header. The decoders
eagerly allocate memory arrays sized proportionally to this unvalidated count
(`e.g. buffered_prefix_length_->Resize(num_prefix * sizeof(int32_t))`).
An attacker can exploit this by crafting a tiny Parquet file (e.g., ~300
bytes) with a maliciously forged `num_values` (e.g., `1,000,000,000`). When
this file is opened (even just to read dictionary pages during
`ParquetFileReader::Open`), Arrow attempts an immediate massive allocation
(e.g., 4 GB), resulting in a `std::bad_alloc` and an immediate Denial of
Service (OOM) crash.
## What changes are included in this PR?
This PR introduces conservative bounds checking in
`cpp/src/parquet/decoder.cc` to ensure that the parsed value counts
conceptually fit within the available byte array buffer size (`bytes_left()`)
before attempting to eagerly allocate memory.
- **`DeltaBitPackDecoder::InitHeader`**: Validates that the requested
`mini_blocks_per_block_` buffer allocation does not exceed the remaining bytes
in the buffer.
- **`DeltaLengthByteArrayDecoder::DecodeLengths`**: Validates that
`num_length` does not exceed a conservative multiplier of the remaining buffer
size (`bytes_left() * 10000`) before calling `Resize()`.
- **`DeltaByteArrayDecoderImpl::SetData`**: Validates that `num_prefix` does
not exceed a conservative multiplier of the remaining buffer size before
calling `Resize()`.
These checks successfully catch maliciously forged massive values in tiny
files while seamlessly permitting legitimate, highly-compressed Parquet files
to be decoded.
## Are these changes tested?
Yes, the changes have been tested against a Proof of Concept (PoC) malicious
file. Instead of triggering an OS OOM or `std::bad_alloc`, the parser now
correctly throws a `ParquetException` ("Excessive num_prefix in
DeltaByteArrayDecoder").
*(Optional: Note if you have included any new C++ unit tests in the PR that
generate a bad header and assert the exception is thrown).*
## Are there any user-facing changes?
No, this strictly improves the security and stability of the Parquet C++
decoders.
---
*Note: This vulnerability was originally submitted to the Huntr bug bounty
platform (Reference ID: `7814255d-e945-427f-ab84-6eddc3a35a37`), and is being
filed here at the recommendation of the ASF Security Team.*
--
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]
