Skip to content

Commit ce2eae0

Browse files
committed
fix(table): bound split count reservations by remaining bytes
Three split decoders read an i32 count from untrusted wire bytes and eagerly reserved Vec capacity from it before reading any element, so a crafted buffer with a valid header and a huge count could reserve hundreds of GB and abort the process via handle_alloc_error. Cap each reservation by the remaining cursor length divided by the minimum per-element wire size (data files: 4B length prefix; deletion entries: 1B null flag; row ranges: 16B), matching the leaf decoders. Capacity is only a hint, so well-formed round-trips are unchanged. Also note paimon_plan_from_split_bytes as a valid plan source in the C bindings.
1 parent 0e32142 commit ce2eae0

2 files changed

Lines changed: 67 additions & 3 deletions

File tree

bindings/c/src/table.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,7 @@ pub unsafe extern "C" fn paimon_plan_from_split_bytes(
519519
///
520520
/// # Safety
521521
/// Only call with a plan returned from `paimon_table_scan_plan`.
522+
/// A plan returned from `paimon_plan_from_split_bytes` is also a valid source.
522523
#[no_mangle]
523524
pub unsafe extern "C" fn paimon_plan_free(plan: *mut paimon_plan) {
524525
if !plan.is_null() {
@@ -533,6 +534,7 @@ pub unsafe extern "C" fn paimon_plan_free(plan: *mut paimon_plan) {
533534
///
534535
/// # Safety
535536
/// `plan` must be a valid pointer from `paimon_table_scan_plan`, or null (returns 0).
537+
/// A plan returned from `paimon_plan_from_split_bytes` is also a valid source.
536538
#[no_mangle]
537539
pub unsafe extern "C" fn paimon_plan_num_splits(plan: *const paimon_plan) -> usize {
538540
if plan.is_null() {

crates/paimon/src/table/source.rs

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,10 @@ impl DataSplit {
736736
source: None,
737737
});
738738
}
739-
let mut data_files = Vec::with_capacity(data_files_n as usize);
739+
// Bound the eager reservation by the remaining bytes: each entry is at least a
740+
// 4-byte length prefix, so a crafted huge count cannot reserve more than the
741+
// buffer could ever hold. Underrun is still caught per-element below.
742+
let mut data_files = Vec::with_capacity((data_files_n as usize).min(cur.len() / 4));
740743
for _ in 0..data_files_n {
741744
let len = read_i32(cur)?;
742745
if len < 0 {
@@ -840,7 +843,9 @@ impl DataSplit {
840843
source: None,
841844
});
842845
}
843-
let mut ranges = Vec::with_capacity(ranges_n as usize);
846+
// Bound the eager reservation by the remaining bytes: each RowRange is
847+
// 16 bytes (two i64), so a crafted huge count cannot over-reserve.
848+
let mut ranges = Vec::with_capacity((ranges_n as usize).min(cur.len() / 16));
844849
for _ in 0..ranges_n {
845850
let from = read_i64(&mut cur)?;
846851
let to = read_i64(&mut cur)?;
@@ -1038,7 +1043,9 @@ fn read_deletion_list(cur: &mut &[u8]) -> crate::Result<Option<Vec<Option<Deleti
10381043
source: None,
10391044
});
10401045
}
1041-
let mut out = Vec::with_capacity(count as usize);
1046+
// Bound the eager reservation by the remaining bytes: each entry is at least a
1047+
// 1-byte null flag, so a crafted huge count cannot over-reserve.
1048+
let mut out = Vec::with_capacity((count as usize).min(cur.len()));
10421049
for _ in 0..count {
10431050
if read_u8(cur)? == 0 {
10441051
out.push(None);
@@ -1572,6 +1579,61 @@ mod tests {
15721579
assert!(DataSplit::deserialize(&bytes).is_err());
15731580
}
15741581

1582+
// A valid v8 header followed by a huge data_files count must return an error, not
1583+
// abort the process via an unbounded `Vec::with_capacity` reservation.
1584+
#[test]
1585+
fn deserialize_rejects_huge_data_files_count_without_aborting() {
1586+
// Build a well-formed split with an empty data-file list so the trailing layout
1587+
// after `data_files_n` is fixed: deletion-null (1) + isStreaming (1) +
1588+
// raw_convertible (1) = 3 bytes. Thus `data_files_n` (i32) sits at `len - 7`.
1589+
let split = DataSplitBuilder::new()
1590+
.with_snapshot(1)
1591+
.with_partition(crate::spec::BinaryRowBuilder::new(0).build())
1592+
.with_bucket(0)
1593+
.with_bucket_path("p".to_string())
1594+
.with_total_buckets(1)
1595+
.with_data_files(vec![])
1596+
.with_raw_convertible(false)
1597+
.build()
1598+
.unwrap();
1599+
let mut bytes = split.serialize().unwrap();
1600+
// Sanity: unpatched buffer round-trips.
1601+
assert!(DataSplit::deserialize(&bytes).is_ok());
1602+
1603+
let pos = bytes.len() - 7;
1604+
bytes[pos..pos + 4].copy_from_slice(&i32::MAX.to_be_bytes());
1605+
match DataSplit::deserialize(&bytes) {
1606+
Err(crate::Error::DataInvalid { .. }) => {}
1607+
other => panic!("expected DataInvalid, got {other:?}"),
1608+
}
1609+
}
1610+
1611+
// Same hardening for the IndexedSplit row-ranges count in the SPLIT_V1 frame.
1612+
#[test]
1613+
fn deserialize_split_v1_rejects_huge_ranges_count_without_aborting() {
1614+
let split = DataSplitBuilder::new()
1615+
.with_snapshot(1)
1616+
.with_partition(crate::spec::BinaryRowBuilder::new(0).build())
1617+
.with_bucket(0)
1618+
.with_bucket_path("p".to_string())
1619+
.with_total_buckets(1)
1620+
.with_data_files(vec![])
1621+
.with_row_ranges(vec![RowRange::new(0, 5)])
1622+
.with_raw_convertible(false)
1623+
.build()
1624+
.unwrap();
1625+
let mut bytes = split.serialize_split_v1().unwrap();
1626+
assert!(DataSplit::deserialize_split_v1(&bytes).is_ok());
1627+
1628+
// Trailing after `ranges_n` (i32): one RowRange (16) + scores flag (1) = 17 bytes.
1629+
let pos = bytes.len() - 21;
1630+
bytes[pos..pos + 4].copy_from_slice(&i32::MAX.to_be_bytes());
1631+
match DataSplit::deserialize_split_v1(&bytes) {
1632+
Err(crate::Error::DataInvalid { .. }) => {}
1633+
other => panic!("expected DataInvalid, got {other:?}"),
1634+
}
1635+
}
1636+
15751637
#[test]
15761638
fn write_java_utf_matches_java_modified_utf8() {
15771639
let enc = |s: &str| {

0 commit comments

Comments
 (0)