From 8a564c1be6915372422e6932c1d36836a378b4b2 Mon Sep 17 00:00:00 2001 From: devtechedge Date: Sat, 24 Oct 2026 03:25:52 +0530 Subject: [PATCH] fix(alp): bounds-check ALP-RD stored metadata and left-parts codes ALP-RD deserialize and decode paths trust stored metadata, so a corrupt file can panic a read instead of returning an error. deserialize bounds-checks the stored dict_len against the stored dictionary, validate rejects dictionaries above MAX_DICT_SIZE and a right_bit_width at or above the float bit width, and both decode paths error on a left-parts code outside the dictionary instead of panicking or decoding garbage. Partially addresses #10756. Signed-off-by: devtechedge --- encodings/alp/src/alp_rd/array.rs | 162 +++++++++++++++++++++++++++++- encodings/alp/src/alp_rd/mod.rs | 16 +++ encodings/alp/src/alp_rd/ops.rs | 41 +++++++- 3 files changed, 215 insertions(+), 4 deletions(-) diff --git a/encodings/alp/src/alp_rd/array.rs b/encodings/alp/src/alp_rd/array.rs index 71a450585a6..8535a556d04 100644 --- a/encodings/alp/src/alp_rd/array.rs +++ b/encodings/alp/src/alp_rd/array.rs @@ -7,6 +7,7 @@ use std::fmt::Formatter; use std::hash::Hash; use std::hash::Hasher; +use ::alp::MAX_DICT_SIZE; use itertools::Itertools; use prost::Message; use vortex_array::Array; @@ -46,6 +47,7 @@ use vortex_buffer::Buffer; use vortex_error::VortexExpect; use vortex_error::VortexResult; use vortex_error::vortex_bail; +use vortex_error::vortex_ensure; use vortex_error::vortex_ensure_eq; use vortex_error::vortex_err; use vortex_error::vortex_panic; @@ -109,6 +111,7 @@ impl VTable for ALPRD { ) -> VortexResult<()> { let alprd_slots = ALPRDSlotsView::from_slots(slots); validate_parts( + data, dtype, len, alprd_slots.left_parts, @@ -179,6 +182,15 @@ impl VTable for ALPRD { ); } + // Stored metadata is untrusted: dict_len must stay within the stored entries. + if metadata.dict_len as usize > metadata.dict.len() { + vortex_bail!( + "ALP-RD dictionary length {} exceeds the {} stored entries", + metadata.dict_len, + metadata.dict.len() + ); + } + let left_parts_dtype = DType::Primitive(metadata.left_parts_ptype(), dtype.nullability()); let left_parts = children.get(0, &left_parts_dtype, len)?; let left_parts_dictionary: Buffer = metadata.dict.as_slice() @@ -470,6 +482,7 @@ fn patches_from_slots( #[allow(clippy::disallowed_methods)] fn validate_parts( + data: &ALPRDData, dtype: &DType, len: usize, left_parts: &ArrayRef, @@ -494,13 +507,35 @@ fn validate_parts( ); } - let expected_right_parts_dtype = match dtype { - DType::Primitive(PType::F32, _) => DType::Primitive(PType::U32, Nullability::NonNullable), - DType::Primitive(PType::F64, _) => DType::Primitive(PType::U64, Nullability::NonNullable), + let (expected_right_parts_dtype, float_bit_width) = match dtype { + DType::Primitive(PType::F32, _) => ( + DType::Primitive(PType::U32, Nullability::NonNullable), + u32::BITS, + ), + DType::Primitive(PType::F64, _) => ( + DType::Primitive(PType::U64, Nullability::NonNullable), + u64::BITS, + ), _ => vortex_bail!("Expected f32 or f64 dtype, got {:?}", dtype), }; vortex_ensure_eq!(right_parts.dtype(), &expected_right_parts_dtype); + // Stored metadata is untrusted: the alp decoder masks codes into an at-most-MAX_DICT_SIZE + // dictionary, and the decode shift overflows the float's bit pattern once right_bit_width + // reaches the width of the float. + vortex_ensure!( + data.left_parts_dictionary().len() <= MAX_DICT_SIZE as usize, + "ALP-RD dictionary holds {} entries, more than MAX_DICT_SIZE ({})", + data.left_parts_dictionary().len(), + MAX_DICT_SIZE + ); + vortex_ensure!( + u32::from(data.right_bit_width()) < float_bit_width, + "ALP-RD right_bit_width {} must be below the {}-bit float width", + data.right_bit_width(), + float_bit_width + ); + if let Some(patches) = left_parts_patches { vortex_ensure_eq!(patches.array_len(), len); // Left-parts exceptions are always all-valid and are stored as the non-nullable left-parts @@ -565,15 +600,24 @@ mod test { use prost::Message; use rstest::rstest; + use vortex_array::ArrayDeserialization; + use vortex_array::ArrayPlugin; + use vortex_array::ArrayRef; + use vortex_array::IntoArray; use vortex_array::VortexSessionExecute; use vortex_array::arrays::PrimitiveArray; use vortex_array::assert_arrays_eq; + use vortex_array::dtype::DType; use vortex_array::dtype::NativePType; + use vortex_array::dtype::Nullability; use vortex_array::dtype::PType; use vortex_array::patches::PatchesMetadata; use vortex_array::test_harness::check_metadata; + use vortex_buffer::buffer; + use vortex_error::VortexResult; use vortex_session::VortexSession; + use super::ALPRD; use super::ALPRDMetadata; use crate::ALPRDFloat; use crate::RDEncoderExt; @@ -633,4 +677,116 @@ mod test { .encode_to_vec(), ); } + + /// Deserialize ALP-RD parts the way a file read does, through the plugin door, so both + /// deserialize-time and validate-time rejections surface as errors. + fn deserialize_alprd( + dtype: &DType, + len: usize, + metadata: &ALPRDMetadata, + children: Vec, + ) -> VortexResult { + ALPRD.deserialize( + ArrayDeserialization::new( + ALPRD.id(), + dtype, + len, + &metadata.encode_to_vec(), + &[], + &children, + ), + &SESSION, + ) + } + + /// The stored dictionary is untrusted, so `dict_len` past the end of the stored entries + /// must be a deserialize error, not a slice panic (#10756). + #[test] + fn test_deserialize_rejects_dict_len_past_the_stored_dictionary() { + let dtype = DType::Primitive(PType::F32, Nullability::NonNullable); + let metadata = ALPRDMetadata { + right_bit_width: 32, + dict_len: 8, + dict: vec![0, 1], + left_parts_ptype: PType::U16 as i32, + patches: None, + }; + let children = vec![buffer![0u16].into_array(), buffer![0u32].into_array()]; + + let error = deserialize_alprd(&dtype, 1, &metadata, children) + .expect_err("dict_len past the stored dictionary must be rejected"); + assert!(error.to_string().contains("dictionary")); + } + + /// The `alp` decoder's lane lookup masks codes into an at-most-`MAX_DICT_SIZE` dictionary, + /// so a longer stored dictionary must be rejected rather than panicking at decode time. + #[test] + fn test_deserialize_rejects_dictionary_larger_than_max_dict_size() { + let dtype = DType::Primitive(PType::F32, Nullability::NonNullable); + let metadata = ALPRDMetadata { + right_bit_width: 16, + dict_len: 9, + dict: vec![0, 1, 2, 3, 4, 5, 6, 7, 8], + left_parts_ptype: PType::U16 as i32, + patches: None, + }; + let children = vec![buffer![0u16].into_array(), buffer![0u32].into_array()]; + + let error = deserialize_alprd(&dtype, 1, &metadata, children) + .expect_err("a dictionary larger than MAX_DICT_SIZE must be rejected"); + assert!(error.to_string().contains("MAX_DICT_SIZE")); + } + + /// The decode shift `left << right_bit_width` overflows the float's bit pattern once + /// `right_bit_width` reaches the width of the float, so the boundary value is rejected. + #[rstest] + #[case::f32(PType::F32, 32)] + #[case::f64(PType::F64, 64)] + fn test_deserialize_rejects_right_bit_width_at_the_float_width( + #[case] float_ptype: PType, + #[case] right_bit_width: u32, + ) { + let dtype = DType::Primitive(float_ptype, Nullability::NonNullable); + let metadata = ALPRDMetadata { + right_bit_width, + dict_len: 2, + dict: vec![0, 1], + left_parts_ptype: PType::U16 as i32, + patches: None, + }; + let right_parts = match float_ptype { + PType::F32 => buffer![0u32].into_array(), + _ => buffer![0u64].into_array(), + }; + let children = vec![buffer![0u16].into_array(), right_parts]; + + let error = deserialize_alprd(&dtype, 1, &metadata, children) + .expect_err("right_bit_width at the float width must be rejected"); + assert!(error.to_string().contains("right_bit_width")); + } + + /// Metadata inside the format's bounds still deserializes and decodes: code 1 maps through + /// the dictionary and the shift stays below the f32 width. + #[test] + fn test_deserialize_accepts_metadata_within_bounds() -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let dtype = DType::Primitive(PType::F32, Nullability::NonNullable); + let metadata = ALPRDMetadata { + right_bit_width: 16, + dict_len: 2, + dict: vec![10, 20], + left_parts_ptype: PType::U16 as i32, + patches: None, + }; + let children = vec![buffer![1u16].into_array(), buffer![0u32].into_array()]; + + let deserialized = deserialize_alprd(&dtype, 1, &metadata, children)?; + let decoded = deserialized.execute::(&mut ctx)?; + assert_arrays_eq!( + decoded, + PrimitiveArray::from_iter([f32::from_bits(20u32 << 16)]), + &mut ctx + ); + Ok(()) + } } diff --git a/encodings/alp/src/alp_rd/mod.rs b/encodings/alp/src/alp_rd/mod.rs index 8b0cd8f2f39..3b897a5c8d7 100644 --- a/encodings/alp/src/alp_rd/mod.rs +++ b/encodings/alp/src/alp_rd/mod.rs @@ -27,6 +27,7 @@ use vortex_buffer::Buffer; use vortex_buffer::BufferMut; use vortex_error::VortexExpect; use vortex_error::VortexResult; +use vortex_error::vortex_bail; use vortex_error::vortex_panic; use vortex_fastlanes::bitpack_compress::bitpack_encode_unchecked; use vortex_session::VortexSession; @@ -153,6 +154,21 @@ where vortex_panic!("alp_rd_decode: left_parts.len != right_parts.len"); } + // Left-parts codes are untrusted: both decode paths below index (or mask into) the + // dictionary with each code, so a code outside it must be an error, not a panic or + // silently decoded garbage. + if let Some(code) = left_parts + .as_slice() + .iter() + .copied() + .find(|&code| code as usize >= left_parts_dict.len()) + { + vortex_bail!( + "ALP-RD left-parts code {code} is not in the {}-entry dictionary", + left_parts_dict.len() + ); + } + if let Some(patches) = left_parts_patches { // Patched path: some left-part codes map to exception values that live outside // the dictionary. We must dictionary-decode first, then overwrite the exceptions, diff --git a/encodings/alp/src/alp_rd/ops.rs b/encodings/alp/src/alp_rd/ops.rs index 024686172c7..55d0acbb733 100644 --- a/encodings/alp/src/alp_rd/ops.rs +++ b/encodings/alp/src/alp_rd/ops.rs @@ -8,6 +8,7 @@ use vortex_array::scalar::Scalar; use vortex_array::vtable::OperationsVTable; use vortex_error::VortexExpect; use vortex_error::VortexResult; +use vortex_error::vortex_err; use crate::ALPRD; use crate::ALPRDArrayExt; @@ -39,7 +40,13 @@ impl OperationsVTable for ALPRD { .as_primitive() .as_::() .vortex_expect("left_code must be non-null"); - array.left_parts_dictionary()[left_code as usize] + let dictionary = array.left_parts_dictionary(); + *dictionary.get(left_code as usize).ok_or_else(|| { + vortex_err!( + "ALP-RD left-parts code {left_code} is not in the {}-entry dictionary", + dictionary.len() + ) + })? } }; @@ -71,14 +78,20 @@ mod test { use std::sync::LazyLock; use rstest::rstest; + use vortex_array::IntoArray; use vortex_array::VortexSessionExecute; use vortex_array::arrays::PrimitiveArray; use vortex_array::assert_arrays_eq; + use vortex_array::dtype::DType; use vortex_array::dtype::NativePType; use vortex_array::dtype::Nullability; + use vortex_array::dtype::PType; use vortex_array::scalar::Scalar; + use vortex_buffer::buffer; + use vortex_error::VortexResult; use vortex_session::VortexSession; + use crate::ALPRD; use crate::ALPRDArrayExt; use crate::ALPRDFloat; use crate::RDEncoder; @@ -145,4 +158,30 @@ mod test { assert_eq!(actual, expected); } } + + /// Left-parts codes are untrusted, and both decode paths index (or mask into) the + /// dictionary with each code, so a code outside the dictionary must be rejected by the + /// first operation that decodes it, rather than panicking or decoding garbage (#10756). + #[test] + fn test_out_of_dictionary_code_rejected_on_first_use() -> VortexResult<()> { + let mut ctx = SESSION.create_execution_ctx(); + let corrupt = ALPRD::try_new( + DType::Primitive(PType::F32, Nullability::NonNullable), + buffer![7u16].into_array(), + buffer![0u16, 1], + buffer![0u32].into_array(), + 16, + None, + )?; + + assert!( + corrupt + .clone() + .into_array() + .execute::(&mut ctx) + .is_err() + ); + assert!(corrupt.execute_scalar(0, &mut ctx).is_err()); + Ok(()) + } }