fix(session): stop RFX geometry from wrapping u16

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<Item = InclusiveRectangle> + '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<Item = Option<InclusiveRectangle>> + '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),
+        })
     })
 }
 

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,
+        ]
+    );
+}