diff --git a/crates/ironrdp-session/src/rfx.rs b/crates/ironrdp-session/src/rfx.rs index a151ff024b..199ec88414 100644 --- a/crates/ironrdp-session/src/rfx.rs +++ b/crates/ironrdp-session/src/rfx.rs @@ -156,6 +156,10 @@ impl DecodingContext { for (update_rectangle, tile_data) in tiles_to_rectangles(tile_set.tiles.as_slice(), destination) .zip(map_tiles_data(tile_set.tiles.as_slice(), tile_set.quants.as_slice())) { + let Some(update_rectangle) = update_rectangle else { + continue; + }; + decode_tile( &tile_data, entropy_algorithm, @@ -233,6 +237,7 @@ fn decode_component( Ok(()) } +#[cfg_attr(feature = "__test", visibility::make(pub))] fn clipping_rectangles( rectangles: &[RfxRectangle], destination: &InclusiveRectangle, @@ -241,28 +246,52 @@ fn clipping_rectangles( ) -> Region { let mut clipping_rectangles = Region::new(); + // Exclusive bounds in u32, so that no server-supplied position or size can wrap. + let right_limit = min(u32::from(destination.left) + u32::from(width), u32::from(u16::MAX) + 1); + let bottom_limit = min(u32::from(destination.top) + u32::from(height), u32::from(u16::MAX) + 1); + rectangles .iter() - .map(|r| InclusiveRectangle { - left: min(destination.left + r.x, destination.left + width - 1), - top: min(destination.top + r.y, destination.top + height - 1), - right: min(destination.left + r.x + r.width - 1, destination.left + width - 1), - bottom: min(destination.top + r.y + r.height - 1, destination.top + height - 1), + .filter_map(|r| { + let left = min(u32::from(destination.left) + u32::from(r.x), right_limit); + let top = min(u32::from(destination.top) + u32::from(r.y), bottom_limit); + let right = min(left + u32::from(r.width), right_limit); + let bottom = min(top + u32::from(r.height), bottom_limit); + + // An empty rectangle covers nothing. + if left == right || top == bottom { + return None; + } + + Some(InclusiveRectangle { + left: u16::try_from(left).ok()?, + top: u16::try_from(top).ok()?, + right: u16::try_from(right - 1).ok()?, + bottom: u16::try_from(bottom - 1).ok()?, + }) }) .for_each(|r| clipping_rectangles.union_rectangle(r)); clipping_rectangles } +/// Returns `None` for a tile that starts past the u16 range, since no clipping rectangle can reach it. +/// A tile crossing the end of the range is cut there, like the clipping rectangles. +#[cfg_attr(feature = "__test", visibility::make(pub))] fn tiles_to_rectangles<'a>( tiles: &'a [Tile<'_>], destination: &'a InclusiveRectangle, -) -> impl Iterator + 'a { - tiles.iter().map(|t| InclusiveRectangle { - left: destination.left + t.x * TILE_SIZE, - top: destination.top + t.y * TILE_SIZE, - right: destination.left + t.x * TILE_SIZE + TILE_SIZE - 1, - bottom: destination.top + t.y * TILE_SIZE + TILE_SIZE - 1, +) -> impl Iterator> + 'a { + tiles.iter().map(|t| { + let left = t.x.checked_mul(TILE_SIZE)?.checked_add(destination.left)?; + let top = t.y.checked_mul(TILE_SIZE)?.checked_add(destination.top)?; + + Some(InclusiveRectangle { + left, + top, + right: left.saturating_add(TILE_SIZE - 1), + bottom: top.saturating_add(TILE_SIZE - 1), + }) }) } diff --git a/crates/ironrdp-testsuite-core/tests/session/rfx.rs b/crates/ironrdp-testsuite-core/tests/session/rfx.rs index a39fc862bf..ab4fdc2eed 100644 --- a/crates/ironrdp-testsuite-core/tests/session/rfx.rs +++ b/crates/ironrdp-testsuite-core/tests/session/rfx.rs @@ -3,6 +3,7 @@ use ironrdp_pdu::ReadCursor; use ironrdp_pdu::geometry::InclusiveRectangle; use ironrdp_session::image::DecodedImage; use ironrdp_session::rfx::DecodingContext; +use rstest::rstest; const IMAGE_WIDTH: usize = 64; const IMAGE_HEIGHT: usize = 64; @@ -1061,3 +1062,133 @@ const DECODED_IMAGE: [u8; IMAGE_WIDTH * IMAGE_HEIGHT * FORMAT_SIZE] = [ 0xFF, 0xF6, 0x9D, 0x13, 0xFF, 0xF6, 0x9C, 0x12, 0xFF, 0xF5, 0x9A, 0x11, 0xFF, 0xF5, 0x9A, 0x11, 0xFF, 0xF5, 0x9A, 0x11, 0xFF, 0xF5, 0x9A, 0x11, 0xFF, ]; + +/// Nothing reaches the image from a tile placed past the u16 range or from an empty rectangle. +#[rstest] +#[case::tile_x(Edit::TileX)] +#[case::tile_y(Edit::TileY)] +#[case::region_width(Edit::RegionWidth)] +#[case::region_height(Edit::RegionHeight)] +#[case::channel_width(Edit::ChannelWidth)] +#[case::channel_height(Edit::ChannelHeight)] +fn decode_draws_nothing_for_unreachable_geometry(#[case] edit: Edit) { + use ironrdp_pdu::codecs::rfx::{Block, CodecChannel}; + + let image = decode_edited_messages(|block| match (edit, block) { + // 1024 * 64 does not fit in u16. + (Edit::TileX, Block::CodecChannel(CodecChannel::TileSet(tile_set))) => tile_set.tiles[0].x = 1024, + (Edit::TileY, Block::CodecChannel(CodecChannel::TileSet(tile_set))) => tile_set.tiles[0].y = 1024, + (Edit::RegionWidth, Block::CodecChannel(CodecChannel::Region(region))) => region.rectangles[0].width = 0, + (Edit::RegionHeight, Block::CodecChannel(CodecChannel::Region(region))) => region.rectangles[0].height = 0, + (Edit::ChannelWidth, Block::Channels(channels)) => channels.0[0].width = 0, + (Edit::ChannelHeight, Block::Channels(channels)) => channels.0[0].height = 0, + _ => {} + }); + + assert!(image.data().iter().all(|&byte| byte == 0)); +} + +#[derive(Clone, Copy)] +enum Edit { + TileX, + TileY, + RegionWidth, + RegionHeight, + ChannelWidth, + ChannelHeight, +} + +/// Decodes [`ENCODED_MESSAGES`] after `edit` has changed its blocks, and returns the image. +fn decode_edited_messages(edit: impl Fn(&mut ironrdp_pdu::codecs::rfx::Block<'_>)) -> DecodedImage { + use ironrdp_pdu::Decode as _; + use ironrdp_pdu::codecs::rfx::Block; + + let destination = InclusiveRectangle { + left: 0, + top: 0, + right: u16::try_from(IMAGE_WIDTH).unwrap() - 1, + bottom: u16::try_from(IMAGE_HEIGHT).unwrap() - 1, + }; + + let mut src = ReadCursor::new(ENCODED_MESSAGES.as_ref()); + let mut encoded = Vec::new(); + while !src.is_empty() { + let mut block = Block::decode(&mut src).unwrap(); + edit(&mut block); + encoded.extend(ironrdp_core::encode_vec(&block).unwrap()); + } + + let mut image = DecodedImage::new( + PixelFormat::BgrX32, + IMAGE_WIDTH.try_into().unwrap(), + IMAGE_HEIGHT.try_into().unwrap(), + ); + DecodingContext::default() + .decode(&mut image, &destination, &mut ReadCursor::new(&encoded)) + .unwrap(); + + image +} + +#[test] +fn clipping_rectangles_reach_the_last_u16_column() { + use ironrdp_pdu::codecs::rfx::RfxRectangle; + use ironrdp_session::rfx::clipping_rectangles; + + let destination = InclusiveRectangle { + left: u16::MAX - 63, + top: u16::MAX - 63, + right: u16::MAX, + bottom: u16::MAX, + }; + // The rectangle and the channel both run 64 pixels past the end of the u16 range. + let rectangles = [RfxRectangle { + x: 0, + y: 0, + width: 128, + height: 128, + }]; + + let clip = clipping_rectangles(&rectangles, &destination, 128, 128); + + assert_eq!(clip.extents, destination); +} + +#[test] +fn tile_rectangle_is_cut_at_the_u16_limit() { + use ironrdp_pdu::codecs::rfx::Tile; + use ironrdp_session::rfx::tiles_to_rectangles; + + let tile = |x| Tile { + y_quant_index: 0, + cb_quant_index: 0, + cr_quant_index: 0, + x, + y: 0, + y_data: &[], + cb_data: &[], + cr_data: &[], + }; + let destination = InclusiveRectangle { + left: 1, + top: 0, + right: u16::MAX, + bottom: 63, + }; + + let rectangles: Vec<_> = tiles_to_rectangles(&[tile(1023), tile(1024)], &destination).collect(); + + // The first tile starts at 1 + 1023 * 64 = 65473 and crosses the limit; the second cannot start in u16. + assert_eq!( + rectangles, + [ + Some(InclusiveRectangle { + left: 65473, + top: 0, + right: u16::MAX, + bottom: 63, + }), + None, + ] + ); +}