diff --git a/crates/openusd/src/usdc/coding.rs b/crates/openusd/src/usdc/coding.rs index b8668bfa..537aabd5 100644 --- a/crates/openusd/src/usdc/coding.rs +++ b/crates/openusd/src/usdc/coding.rs @@ -19,7 +19,9 @@ pub fn encoded_buffer_size(count: usize) -> usize { 0 } else { let sz = mem::size_of::(); - sz + (count * 2).div_ceil(8) + (sz * count) + // Saturating: a count from a damaged file must not overflow here. + sz.saturating_add(count.saturating_mul(2).div_ceil(8)) + .saturating_add(sz.saturating_mul(count)) } } @@ -45,15 +47,22 @@ where codes_reader.read_pod::()? as i64 }; - let num_code_bytes = (count * 2).div_ceil(8); + let num_code_bytes = count + .checked_mul(2) + .ok_or_else(|| ReadError::corrupt(format!("integer count {count} overflows")))? + .div_ceil(8); let mut ints_reader = { let offset = mem::size_of::() + num_code_bytes; - io::Cursor::new(&data[offset..]) + let rest = data + .get(offset..) + .ok_or_else(|| ReadError::corrupt(format!("{count} integers need more than {} bytes", data.len())))?; + io::Cursor::new(rest) }; let mut prev = 0_i64; - let mut output = Vec::with_capacity(count); + // Every integer takes at least two bits of `data`. + let mut output = Vec::with_capacity(count.min(data.len().saturating_mul(4))); for _ in 0..num_code_bytes { // Code byte stores integer types for the next 4 integers. diff --git a/crates/openusd/src/usdc/reader.rs b/crates/openusd/src/usdc/reader.rs index 6ca4f5ac..63626bd6 100644 --- a/crates/openusd/src/usdc/reader.rs +++ b/crates/openusd/src/usdc/reader.rs @@ -34,6 +34,10 @@ macro_rules! corrupt { }; } +/// Largest single LZ4-decompressed block the reader allocates (4 GiB). +/// (Saturates on 32-bit targets.) +const MAX_DECOMPRESSED_BYTES: usize = if usize::BITS > 32 { 4 << 30 } else { usize::MAX }; + // Maximum supported USDC crate version. // See USD Core Specification v1.0.1 §16.3.8.2 for version history: // 0.10.0 — Path Expression value types @@ -46,6 +50,8 @@ const SW_VERSION: Version = version(0, 12, 0); pub struct CrateFile { /// File reader. reader: R, + /// Length of the whole file: what bounds every count read from it. + file_length: usize, /// File header. pub bootstrap: Bootstrap, @@ -84,9 +90,13 @@ impl CrateFile { /// Read structural sections of a crate file. pub fn open(mut reader: R) -> Result { let bootstrap = Self::read_header(&mut reader)?; + let here = reader.stream_position()?; + let file_length = reader.seek(io::SeekFrom::End(0))? as usize; + reader.seek(io::SeekFrom::Start(here))?; let mut file = CrateFile { reader, + file_length, bootstrap, sections: Vec::new(), tokens: Vec::new(), @@ -212,7 +222,7 @@ impl CrateFile { let count = self.reader.read_count()?; self.tokens = if file_ver < version(0, 4, 0) { - todo!("Support TOKENS reader for < 0.4.0 files"); + return Err(ReadError::unsupported("Support TOKENS reader for < 0.4.0 files")); } else { let uncompressed_size = self.reader.read_count()?; let mut buffer = self.read_compressed(uncompressed_size)?; @@ -289,7 +299,7 @@ impl CrateFile { let file_ver = self.version(); self.fields = if file_ver < version(0, 4, 0) { - todo!("Support FIELDS reader before < 0.4.0") + return Err(ReadError::unsupported("Support FIELDS reader before < 0.4.0")); } else { let field_count = self.reader.read_count()?; @@ -305,7 +315,7 @@ impl CrateFile { .map(|(index, value)| Field::new(*index, *value)) .collect(); - debug_assert_eq!(fields.len(), field_count); + corrupt!(fields.len() == field_count); fields }; @@ -323,7 +333,7 @@ impl CrateFile { let file_ver = self.version(); self.fieldsets = if file_ver < version(0, 4, 0) { - todo!("Support FIELDSETS reader for < 0.4.0 files"); + return Err(ReadError::unsupported("Support FIELDSETS reader for < 0.4.0 files")); } else { let count = self.reader.read_count()?; @@ -336,7 +346,7 @@ impl CrateFile { .map(|i| if i == INVALID_INDEX { None } else { Some(i as usize) }) .collect::>(); - debug_assert_eq!(sets.len(), count); + corrupt!(sets.len() == count); sets }; @@ -354,12 +364,17 @@ impl CrateFile { let file_ver = self.version(); if file_ver == version(0, 0, 1) { - todo!("Support PATHS reader for == 0.0.1 files"); + return Err(ReadError::unsupported("Support PATHS reader for == 0.0.1 files")); } else if file_ver < version(0, 4, 0) { - todo!("Support PATHS reader for < 0.4.0 files"); + return Err(ReadError::unsupported("Support PATHS reader for < 0.4.0 files")); } else { // Read # of paths. let path_count = self.reader.read_count()?; + corrupt!( + path_count <= self.file_length, + "path count {path_count} is larger than a {}-byte file can hold", + self.file_length + ); self.paths = vec![sdf::Path::default(); path_count]; self.read_compressed_paths()?; @@ -376,13 +391,13 @@ impl CrateFile { // Read compressed data. let path_indexes = self.read_encoded_ints::(count)?; - debug_assert_eq!(path_indexes.len(), count); + corrupt!(path_indexes.len() == count); let element_token_indexes = self.read_encoded_ints::(count)?; - debug_assert_eq!(element_token_indexes.len(), count); + corrupt!(element_token_indexes.len() == count); let jumps = self.read_encoded_ints::(count)?; - debug_assert_eq!(jumps.len(), count); + corrupt!(jumps.len() == count); self.build_compressed_paths(&path_indexes, &element_token_indexes, &jumps)?; @@ -492,9 +507,9 @@ impl CrateFile { let file_ver = self.version(); self.specs = if file_ver == version(0, 0, 1) { - todo!("Support SPECS reader for == 0.0.1 files"); + return Err(ReadError::unsupported("Support SPECS reader for == 0.0.1 files")); } else if file_ver < version(0, 4, 0) { - todo!("Support SPECS reader for < 0.4.0 files"); + return Err(ReadError::unsupported("Support SPECS reader for < 0.4.0 files")); } else { // Version 0.4.0 specs are compressed @@ -592,11 +607,25 @@ impl CrateFile { ) -> Result, ReadError> { // Read data to memory. let compressed_size = self.reader.read_count()?; + corrupt!( + compressed_size <= self.file_length, + "compressed size {compressed_size} is larger than the {}-byte file", + self.file_length + ); let mut input = vec![0_u8; compressed_size]; self.reader.read_exact(&mut input)?; - // Decompress to output buffer. - let mut output = vec![T::default(); estimated_count]; + // Decompress to a buffer no larger than LZ4 can expand this input + // (`estimated_count` is an upper bound from the file, not a size). + // A single decoded value over MAX_DECOMPRESSED_BYTES is refused like + // the other suspiciously large counts: LZ4 alone would allow 255x a + // compressed block that may be most of the file. + let most = compressed_size + .saturating_mul(255) + .saturating_add(64) + .min(MAX_DECOMPRESSED_BYTES) + / mem::size_of::().max(1); + let mut output = vec![T::default(); estimated_count.min(most)]; let actual_size = decompress_lz4(&input, cast_slice_mut(&mut output))?; let actual_count = actual_size / mem::size_of::(); @@ -618,7 +647,7 @@ impl CrateFile { let buffer = self.read_compressed::(estimated_size)?; let ints = coding::decode_ints(buffer.as_slice(), count)?; - debug_assert_eq!(ints.len(), count); + corrupt!(ints.len() == count); Ok(ints) } @@ -627,7 +656,7 @@ impl CrateFile { // Implements various logic and compatibility checks to figure out the array length and whether it's compressed. fn unpack_array_len(&mut self, value: ValueRep, kind: ArrayKind) -> Result<(usize, bool), ReadError> { - debug_assert!(!value.is_inlined()); + corrupt!(!value.is_inlined()); // Empty array. if value.payload() == 0 { @@ -661,7 +690,7 @@ impl CrateFile { ArrayKind::Other => { // Fallback to uncompressed. // See https://github.com/PixarAnimationStudios/OpenUSD/blob/0b18ad3f840c24eb25e16b795a5b0821cf05126e/pxr/usd/usd/crateFile.cpp#L1868 - debug_assert!(!value.is_compressed()); + corrupt!(!value.is_compressed()); compressed = false; } } @@ -729,7 +758,9 @@ impl CrateFile { let mut output = vec![T::zero(); count]; for (i, index) in indexes.into_iter().enumerate() { - output[i] = lut[index as usize]; + output[i] = *lut.get(index as usize).ok_or_else(|| { + ReadError::corrupt(format!("lookup index {index} is outside a {}-entry table", lut.len())) + })?; } output @@ -954,7 +985,7 @@ impl CrateFile { /// the on-disk element — safe for all gf vec types since they are /// `#[repr(C)]` and `bytemuck::Pod`. fn read_gf_array(&mut self, value: ValueRep) -> Result, ReadError> { - debug_assert!(value.is_array() && !value.is_compressed()); + corrupt!(value.is_array() && !value.is_compressed()); let (count, _) = self.unpack_array_len(value, ArrayKind::Other)?; if count == 0 { return Ok(Vec::default()); @@ -966,8 +997,8 @@ impl CrateFile { &mut self, value: ValueRep, ) -> Result, ReadError> { - debug_assert!(value.is_array()); - debug_assert!(!value.is_compressed()); + corrupt!(value.is_array()); + corrupt!(!value.is_compressed()); let (count, _) = self.unpack_array_len(value, ArrayKind::Other)?; @@ -1282,7 +1313,7 @@ impl CrateFile { let list = self.read_list_op(value, |file: &mut Self| { let count = file.reader.read_count()?; - let mut vec = Vec::with_capacity(count); + let mut vec = Vec::with_capacity(count.min(1024)); for _ in 0..count { let reference = file.read_reference()?; @@ -1384,7 +1415,7 @@ impl CrateFile { Type::PayloadListOp => { let list = self.read_list_op(value, |file: &mut Self| { let count = file.reader.read_count()?; - let mut vec = Vec::with_capacity(count); + let mut vec = Vec::with_capacity(count.min(1024)); for _ in 0..count { let payload = file.read_payload()?; vec.push(payload); @@ -1404,7 +1435,7 @@ impl CrateFile { self.set_position(value.payload())?; let count = self.reader.read_count()?; - let mut map = HashMap::with_capacity(count); + let mut map = HashMap::with_capacity(count.min(1024)); for _ in 0..count { let key = self.read_string()?; @@ -1448,7 +1479,7 @@ impl CrateFile { corrupt!(count == times.len(), "Invalid time samples count"); let value_reps = self.reader.read_vec::(count)?; - debug_assert_eq!(value_reps.len(), count); + corrupt!(value_reps.len() == count); let samples = times .into_iter() @@ -1523,7 +1554,7 @@ impl CrateFile { corrupt!(!value.is_inlined()); self.set_position(value.payload())?; let count = self.reader.read_count()?; - let mut pairs = Vec::with_capacity(count); + let mut pairs = Vec::with_capacity(count.min(1024)); for _ in 0..count { let src_idx: u32 = self.reader.read_pod()?; let tgt_idx: u32 = self.reader.read_pod()?; @@ -1606,7 +1637,7 @@ fn decompress_lz4(mut input: &[u8], output: &mut [u8]) -> Result ReadExt for R { return Ok(Vec::new()); } + // Read what the stream really holds before trusting `count`: a + // truncated or lying file fails here instead of allocating first. + let bytes = count + .checked_mul(mem::size_of::()) + .ok_or_else(|| ReadError::corrupt(format!("vector of {count} elements overflows")))?; + let mut raw = Vec::new(); + io::Read::read_to_end(&mut io::Read::take(&mut *self, bytes as u64), &mut raw).ctx("vec")?; + if raw.len() != bytes { + return Err(ReadError::corrupt(format!( + "vec: {} of {bytes} bytes before the end", + raw.len() + ))); + } let mut vec = vec![T::default(); count]; - self.read_exact(cast_slice_mut(&mut vec)).ctx("vec")?; - + cast_slice_mut(&mut vec).copy_from_slice(&raw); Ok(vec) } } diff --git a/crates/openusd/tests/usdc_malformed.rs b/crates/openusd/tests/usdc_malformed.rs new file mode 100644 index 00000000..0855ef71 --- /dev/null +++ b/crates/openusd/tests/usdc_malformed.rs @@ -0,0 +1,99 @@ +//! Damaged `.usdc` input must come back as an error, never a panic, an +//! allocation failure or a stack overflow: an application built with +//! `panic = "abort"` would otherwise crash on a bad file. Each case mutates +//! a known-good fixture deterministically and reads every field value. + +use std::io::Cursor; +use std::panic; + +use openusd::sdf::AbstractData; +use openusd::usdc::CrateData; + +const FIXTURES: [&str; 5] = [ + "reference.usdc", + "ints.usdc", + "sdf_types.usdc", + "payload.usdc", + "floats.usdc", +]; + +/// xorshift64: the same mutations on every run and platform. +struct Rng(u64); + +impl Rng { + fn next(&mut self) -> u64 { + self.0 ^= self.0 << 13; + self.0 ^= self.0 >> 7; + self.0 ^= self.0 << 17; + self.0 + } + + fn below(&mut self, n: usize) -> usize { + (self.next() % n as u64) as usize + } +} + +/// One of: flipped bits, an extreme 64-bit value (the shape of a lying +/// count or offset), a truncation, or a large 32-bit value. +fn mutate(bytes: &mut Vec, rng: &mut Rng) { + match rng.next() % 4 { + 0 => { + for _ in 0..1 + rng.next() % 8 { + let i = rng.below(bytes.len()); + bytes[i] ^= 1 << (rng.next() % 8); + } + } + 1 => { + let i = rng.below(bytes.len() - 8); + let value = [u64::MAX, i64::MAX as u64, u32::MAX as u64, 1 << 30][rng.below(4)]; + bytes[i..i + 8].copy_from_slice(&value.to_le_bytes()); + } + 2 => { + let len = rng.below(bytes.len()).max(1); + bytes.truncate(len); + } + _ => { + let i = rng.below(bytes.len() - 4); + bytes[i..i + 4].copy_from_slice(&((rng.next() as u32) | 0x1000_0000).to_le_bytes()); + } + } +} + +/// Opens the crate and decodes every value, as a stage would. +fn read_everything(bytes: Vec) { + let Ok(data) = CrateData::open(Cursor::new(bytes), true) else { + return; + }; + for path in data.spec_paths() { + for field in data.list_fields(&path).unwrap_or_default() { + let _ = data.try_field(&path, &field); + } + } +} + +#[test] +fn mutated_crate_files_are_errors_not_panics() { + let rounds: usize = std::env::var("OPENUSD_USDC_MUTATIONS") + .ok() + .and_then(|value| value.parse().ok()) + .unwrap_or(2_000); + let mut rng = Rng(0x2545_F491_4F6C_DD1D); + let mut panics = Vec::new(); + for fixture in FIXTURES { + let path = format!("{}/fixtures/{fixture}", env!("CARGO_MANIFEST_DIR")); + let seed = std::fs::read(&path).unwrap_or_else(|e| panic!("cannot read {path}: {e}")); + for round in 0..rounds { + let mut bytes = seed.clone(); + mutate(&mut bytes, &mut rng); + if panic::catch_unwind(|| read_everything(bytes)).is_err() { + panics.push(format!("{fixture} round {round}")); + } + } + } + assert!( + panics.is_empty(), + "{} panics, first: {:?}", + panics.len(), + &panics[..panics.len().min(5)] + ); +}