Skip to content

Commit b7fe7d1

Browse files
authored
fix: reject truncated BinaryRow serialized bytes instead of panicking (#364)
1 parent f0bbd0a commit b7fe7d1

1 file changed

Lines changed: 74 additions & 2 deletions

File tree

crates/paimon/src/spec/binary_row.rs

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,29 @@ impl BinaryRow {
9292
});
9393
}
9494
let arity = i32::from_be_bytes([data[0], data[1], data[2], data[3]]);
95-
Ok(Self::from_bytes(arity, data[4..].to_vec()))
95+
if arity < 0 {
96+
return Err(crate::Error::UnexpectedError {
97+
message: format!("BinaryRow: serialized data has negative arity: {arity}"),
98+
source: None,
99+
});
100+
}
101+
let body = &data[4..];
102+
// The body must hold at least the null bitmap and the fixed part
103+
// (8 bytes per field); reject truncated input rather than panicking
104+
// later when reading the null bitmap or a field. The size is computed
105+
// in i64 so an absurd arity in malformed input cannot overflow.
106+
let bit_set_width = ((arity as i64 + 63 + Self::HEADER_SIZE_IN_BYTES as i64) / 64) * 8;
107+
let fix_part_size = bit_set_width + 8 * arity as i64;
108+
if (body.len() as i64) < fix_part_size {
109+
return Err(crate::Error::UnexpectedError {
110+
message: format!(
111+
"BinaryRow: serialized body too short for arity {arity}: {} bytes, need at least {fix_part_size}",
112+
body.len()
113+
),
114+
source: None,
115+
});
116+
}
117+
Ok(Self::from_bytes(arity, body.to_vec()))
96118
}
97119

98120
/// Serialize this BinaryRow to bytes (arity prefix + data), the inverse of `from_serialized_bytes`.
@@ -119,7 +141,13 @@ impl BinaryRow {
119141
let bit_index = pos + Self::HEADER_SIZE_IN_BYTES as usize;
120142
let byte_index = bit_index / 8;
121143
let bit_offset = bit_index % 8;
122-
(self.data[byte_index] & (1 << bit_offset)) != 0
144+
// Index defensively: a truncated buffer that lacks the null bitmap
145+
// byte is reported as not-null so the typed field readers can return
146+
// a graceful error instead of this method panicking.
147+
match self.data.get(byte_index) {
148+
Some(byte) => (byte & (1 << bit_offset)) != 0,
149+
None => false,
150+
}
123151
}
124152

125153
fn field_offset(&self, pos: usize) -> usize {
@@ -1186,6 +1214,50 @@ mod tests {
11861214
assert!(BinaryRow::from_serialized_bytes(&[0, 0]).is_err());
11871215
}
11881216

1217+
#[test]
1218+
fn test_from_serialized_bytes_truncated_body() {
1219+
// Valid 4-byte arity prefix (arity = 1) but the body is empty, so it
1220+
// cannot hold the null bitmap. This must be rejected gracefully rather
1221+
// than panicking when the null bitmap is later read.
1222+
let truncated = [0u8, 0, 0, 1];
1223+
assert!(BinaryRow::from_serialized_bytes(&truncated).is_err());
1224+
1225+
// Body present but still shorter than the fixed part (null bitmap of 8
1226+
// bytes + one 8-byte field = 16 bytes for arity 1).
1227+
let mut short_body = vec![0u8, 0, 0, 1];
1228+
short_body.extend_from_slice(&[0u8; 4]);
1229+
assert!(BinaryRow::from_serialized_bytes(&short_body).is_err());
1230+
}
1231+
1232+
#[test]
1233+
fn test_from_serialized_bytes_negative_arity() {
1234+
// arity = -1 (0xFFFFFFFF) must be rejected, not used in size math.
1235+
let data = [0xFFu8, 0xFF, 0xFF, 0xFF, 0, 0, 0, 0];
1236+
assert!(BinaryRow::from_serialized_bytes(&data).is_err());
1237+
}
1238+
1239+
#[test]
1240+
fn test_from_serialized_bytes_well_formed_decodes() {
1241+
// Negative control: a correctly sized body decodes and reads back fine.
1242+
let mut builder = BinaryRowBuilder::new(1);
1243+
builder.write_int(0, 7);
1244+
let serialized = builder.build_serialized();
1245+
let row = BinaryRow::from_serialized_bytes(&serialized).unwrap();
1246+
assert_eq!(row.arity(), 1);
1247+
assert!(!row.is_null_at(0));
1248+
assert_eq!(row.get_int(0).unwrap(), 7);
1249+
}
1250+
1251+
#[test]
1252+
fn test_is_null_at_short_buffer_does_not_panic() {
1253+
// A row whose backing buffer lacks the null bitmap byte must not panic
1254+
// in is_null_at; the position is reported as not-null and the typed
1255+
// reader then returns a graceful error.
1256+
let row = BinaryRow::from_bytes(1, Vec::new());
1257+
assert!(!row.is_null_at(0));
1258+
assert!(row.get_int(0).is_err());
1259+
}
1260+
11891261
#[test]
11901262
fn test_get_int() {
11911263
let mut builder = BinaryRowBuilder::new(2);

0 commit comments

Comments
 (0)