This is an automated email from the ASF dual-hosted git repository.
etseidl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git
The following commit(s) were added to refs/heads/main by this push:
new 7e3b403ac6 fix(parquet): reject Thrift list sizes larger than
remaining input (#10979)
7e3b403ac6 is described below
commit 7e3b403ac6481493b169c6f83144070f6352026b
Author: Marcelo Tesla <[email protected]>
AuthorDate: Fri Sep 4 11:34:56 2026 -0300
fix(parquet): reject Thrift list sizes larger than remaining input (#10979)
# Which issue does this PR close?
- Closes #10920.
# Rationale for this change
`read_thrift_vec` reserved `Vec` capacity from the compact list header's
declared element count before reading any element. A 23-byte footer
whose `schema` list claimed ~109 million `SchemaElement`s asked for ~10
GB. Allocation failure aborts the process; `ParquetMetaDataReader` never
returns an error.
Each list element occupies at least one byte on the wire, so the
declared size can be bounded by the bytes remaining in the metadata
slice. That slice is already length-limited by the footer; this check is
the level below.
# What changes are included in this PR?
- `ThriftCompactInputProtocol::remaining_bytes` reports unread length
for slice-backed input (`ThriftSliceInputProtocol`). Stream-backed input
(`ThriftReadInputProtocol`) leaves it unset.
- `read_thrift_vec` returns `ParquetError::General` when the declared
size exceeds remaining input, and only then calls `Vec::with_capacity`.
No public API change.
# Are these changes tested?
Unit tests in `parquet_thrift`:
- well-formed 2-element `i32` list still decodes
- 14-element header with no payload returns an error
- declared size 109_002_364 with two leftover bytes returns an error
(the schema-list case from the issue)
`cargo test -p parquet --lib parquet_thrift` and `cargo clippy -p
parquet --lib --tests --all-features -- -D warnings`.
# Are there any user-facing changes?
Malformed footers that previously aborted now return `ParquetError`.
Valid files are unchanged.
# AI Disclosure
Assisted draft of the remaining-bytes check and regression tests.
Reviewed against the compact protocol list encoding and verified with
the tests above.
Co-authored-by: Marcelo Tesla <[email protected]>
---
parquet/src/parquet_thrift.rs | 69 ++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 68 insertions(+), 1 deletion(-)
diff --git a/parquet/src/parquet_thrift.rs b/parquet/src/parquet_thrift.rs
index 50a1235ce3..9e69fc382b 100644
--- a/parquet/src/parquet_thrift.rs
+++ b/parquet/src/parquet_thrift.rs
@@ -297,6 +297,14 @@ pub(crate) trait ThriftCompactInputProtocol<'a> {
/// Skip the next `n` bytes of input.
fn skip_bytes(&mut self, n: usize) -> ThriftProtocolResult<()>;
+ /// Remaining unread bytes, if this protocol is backed by a finite buffer.
+ ///
+ /// Used to reject Thrift collection sizes that cannot fit in the remaining
+ /// input before allocating.
+ fn remaining_bytes(&self) -> Option<usize> {
+ None
+ }
+
/// Read a ULEB128 encoded unsigned varint from the input.
fn read_vlq(&mut self) -> ThriftProtocolResult<u64> {
// try the happy path first
@@ -598,6 +606,10 @@ impl<'b, 'a: 'b> ThriftCompactInputProtocol<'b> for
ThriftSliceInputProtocol<'a>
Err(_) => unreachable!(),
}
}
+
+ fn remaining_bytes(&self) -> Option<usize> {
+ Some(self.buf.len())
+ }
}
/// A Thrift input protocol that wraps a [`Read`] object.
@@ -721,7 +733,20 @@ where
{
let list_ident = prot.read_list_begin()?;
validate_list_type(T::ELEMENT_TYPE, &list_ident)?;
- let mut res = Vec::with_capacity(list_ident.size as usize);
+ let size = list_ident.size as usize;
+ // Each list element occupies at least one byte on the wire. Bound the
+ // declared count by remaining input before reserving, so a malformed
+ // header cannot abort the process with a huge allocation.
+ if let Some(remaining) = prot.remaining_bytes()
+ && size > remaining
+ {
+ return Err(general_err!(
+ "Thrift list size {} exceeds remaining input length {}",
+ size,
+ remaining
+ ));
+ }
+ let mut res = Vec::with_capacity(size);
for _ in 0..list_ident.size {
let val = T::read_thrift(prot)?;
res.push(val);
@@ -1215,4 +1240,46 @@ pub(crate) mod tests {
.contains("Expected list element type of I32 but got Bool")
);
}
+
+ #[test]
+ fn test_read_thrift_vec_roundtrip_i32() {
+ // 2-element list of i32: header 0x25 (count=2, type=I32), two zigzag
zeros.
+ let data = [0x25, 0x00, 0x00];
+ let mut prot = ThriftSliceInputProtocol::new(&data);
+ let result = read_thrift_vec::<i32, ThriftSliceInputProtocol>(&mut
prot).unwrap();
+ assert_eq!(result, vec![0, 0]);
+ }
+
+ #[test]
+ fn test_read_thrift_vec_size_exceeds_remaining_returns_err() {
+ // Header 0xE5: 14 i32 elements, no payload. After reading the header
+ // zero bytes remain, so this must error instead of reserving 14 slots
+ // (and, for a larger declared size, gigabytes).
+ let data = [0xE5];
+ let mut prot = ThriftSliceInputProtocol::new(&data);
+ let result = read_thrift_vec::<i32, ThriftSliceInputProtocol>(&mut
prot);
+ assert!(result.is_err(), "expected error, got {result:?}");
+ assert!(
+ result
+ .unwrap_err()
+ .to_string()
+ .contains("Thrift list size 14 exceeds remaining input length
0")
+ );
+ }
+
+ #[test]
+ fn test_read_thrift_vec_huge_declared_size_returns_err() {
+ // Compact list header: 0xF5 = I32 elements, size follows as a varint.
+ // 0xfc 0xfc 0xfc 0x33 decodes to 109_002_364 — the schema-list case
+ // from #10920 — with only two leftover bytes.
+ let data = [0xF5, 0xfc, 0xfc, 0xfc, 0x33, 0x00, 0x00];
+ let mut prot = ThriftSliceInputProtocol::new(&data);
+ let result = read_thrift_vec::<i32, ThriftSliceInputProtocol>(&mut
prot);
+ assert!(result.is_err(), "expected error, got {result:?}");
+ let err = result.unwrap_err().to_string();
+ assert!(
+ err.contains("Thrift list size 109002364 exceeds remaining input
length 2"),
+ "{err}"
+ );
+ }
}
