diff --git a/crates/exr/src/decode.rs b/crates/exr/src/decode.rs index ebadd1c79..ab21064ce 100644 --- a/crates/exr/src/decode.rs +++ b/crates/exr/src/decode.rs @@ -420,8 +420,9 @@ pub struct DisplayPart { /// Decodes the kept flat channels of one part and places them in its display window. /// /// Channels are decoded in their stored sample type, converted to `f32`, and clipped or padded -/// according to their data and display windows. `fill` receives each stored channel name. Both -/// the decoded data and display-window buffers are bounded by `limits` before allocation. +/// according to their data and display windows. `keep` and `fill` receive the part-qualified name +/// from [`part_channel_name`], which is also returned in [`DisplayChannel::name`]. Both decoded +/// block data and display-window buffers are bounded by `limits` before allocation. pub fn decode_display_part( bytes: &[u8], index: usize, @@ -456,8 +457,11 @@ pub fn decode_display_part( if bytes_needed > limits.max_bytes || usize::try_from(bytes_needed).is_err() { return Err(Error::LimitExceeded(format!("{bytes_needed} display-window bytes exceed {}", limits.max_bytes))); } - let select = DisplaySelect { index, keep, fill, limits, seen: std::cell::Cell::new(0) }; - let image = ReadImage::new(ReadAllLayers { read_channels: select }, |_| {}).from_buffered(Cursor::new(bytes)).map_err(map_exr)?; + let block_limit = std::cell::Cell::new(None); + let select = DisplaySelect { index, keep, fill, limits, seen: std::cell::Cell::new(0), block_limit: &block_limit }; + let image = ReadImage::new(ReadAllLayers { read_channels: select }, |_| {}) + .from_buffered(Cursor::new(bytes)) + .map_err(|error| block_limit.take().map(Error::LimitExceeded).unwrap_or_else(|| map_exr(error)))?; let channels = image .layer_data .into_iter() @@ -478,6 +482,7 @@ struct DisplaySelect<'a> { fill: &'a dyn Fn(&str) -> f32, limits: &'a Limits, seen: std::cell::Cell, + block_limit: &'a std::cell::Cell>, } struct DisplaySelectReader { @@ -510,6 +515,20 @@ impl<'s> ReadChannels<'s> for DisplaySelect<'_> { u32s: vec![], }); } + // The EXR crate decompresses each accepted block before `read_block` can discard ignored + // channels. Bound that all-channel input separately from the selected output planes. + let Some(block_bytes) = layer_w + .checked_mul(layer_h) + .and_then(|pixels| pixels.checked_mul(header.channels.list.len().max(1))) + .and_then(|samples| samples.checked_mul(std::mem::size_of::())) + else { + self.block_limit.set(Some(format!("data-window block size exceeds {}", self.limits.max_bytes))); + return Err(exr::error::Error::Invalid("data-window block size overflow".into())); + }; + if u64::try_from(block_bytes).map_or(true, |bytes| bytes > self.limits.max_bytes) || block_bytes > isize::MAX as usize { + self.block_limit.set(Some(format!("{block_bytes} data-window bytes exceed {}", self.limits.max_bytes))); + return Err(exr::error::Error::Invalid("data-window blocks exceed limit".into())); + } let pixels = display_w.checked_mul(display_h).filter(|n| *n > 0).ok_or(exr::error::Error::Invalid("display window".into()))?; let prefix = header.own_attributes.layer_name.as_ref().map(|name| name.to_string()).filter(|name| !name.is_empty()); let kept = @@ -524,7 +543,14 @@ impl<'s> ReadChannels<'s> for DisplaySelect<'_> { let mut planes = Vec::with_capacity(header.channels.list.len()); for channel in header.channels.list.iter() { let name = part_channel_name(prefix.as_deref(), &channel.name.to_string()); - let plane = if (self.keep)(&name) { Some((name.clone(), vec![(self.fill)(&name); pixels])) } else { None }; + let plane = if (self.keep)(&name) { + if channel.sampling.0 != 1 || channel.sampling.1 != 1 { + return Err(exr::error::Error::NotSupported("subsampled channels".into())); + } + Some((name.clone(), vec![(self.fill)(&name); pixels])) + } else { + None + }; planes.push(plane); } let position = header.own_attributes.layer_position; diff --git a/crates/exr/src/decode_tests.rs b/crates/exr/src/decode_tests.rs index e02e7b548..54c28d9aa 100644 --- a/crates/exr/src/decode_tests.rs +++ b/crates/exr/src/decode_tests.rs @@ -128,6 +128,11 @@ fn decoded_display_part_places_selected_channels_and_checks_total_budget() { let limits = Limits { max_bytes: 6 * 4 - 1, ..Limits::default() }; assert!(matches!(decode_display_part(&bytes, 0, &all, &|_| 0.0, &limits), Err(Error::LimitExceeded(_)))); + + // One selected display plane fits exactly, but the EXR decoder must decompress both data + // channels in its input block before discarding the ignored one. + let block_limits = Limits { max_bytes: 6 * 4, ..Limits::default() }; + assert!(matches!(decode_display_part(&bytes, 0, &|name| name == "R", &|_| 0.0, &block_limits), Err(Error::LimitExceeded(_)))); } #[test] @@ -139,6 +144,20 @@ fn decoded_display_part_clips_a_wide_data_window_to_the_small_display() { assert_eq!(part.channels[0].samples, [0.0]); } +#[test] +fn decoded_display_part_rejects_kept_subsampled_channels() { + let mut bytes = file(vec![layer(None, 4, 1, &["R"], false, exr::prelude::Compression::Uncompressed)], (4, 1)); + // exr's writer doesn't emit subsampling yet, so patch the standard chlist sampling field in a + // valid uncompressed file. The reader must reject it before looking at the block payload. + let tag = b"channels\0chlist\0"; + let attr = bytes.windows(tag.len()).position(|window| window == tag).unwrap(); + let payload = attr + tag.len() + 4; + let name_end = payload + bytes[payload..].iter().position(|byte| *byte == 0).unwrap(); + let x_sampling = name_end + 1 + 4 + 1 + 3; + bytes[x_sampling..x_sampling + 4].copy_from_slice(&2u32.to_le_bytes()); + assert!(matches!(decode_display_part(&bytes, 0, &|name| name == "R", &|_| 0.0, &Limits::default()), Err(Error::Unsupported(_)))); +} + /// A 2x2 single-part file whose layer carries a 2x1 preview. fn with_preview() -> Vec { let mut attrs = LayerAttributes::named("rgb");