diff --git a/crates/ironrdp-session/src/rfx.rs b/crates/ironrdp-session/src/rfx.rs index a151ff024b..abb092b0a9 100644 --- a/crates/ironrdp-session/src/rfx.rs +++ b/crates/ironrdp-session/src/rfx.rs @@ -154,7 +154,7 @@ impl DecodingContext { let mut final_update_rectangle = clipping_rectangles.extents.clone(); 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())) + .zip(map_tiles_data(tile_set.tiles.as_slice(), tile_set.quants.as_slice())?) { decode_tile( &tile_data, @@ -266,16 +266,28 @@ fn tiles_to_rectangles<'a>( }) } -fn map_tiles_data<'a>(tiles: &[Tile<'a>], quants: &[Quant]) -> Vec> { +fn map_tiles_data<'a>(tiles: &[Tile<'a>], quants: &[Quant]) -> SessionResult>> { + let quant = |index: u8| { + quants.get(usize::from(index)).cloned().ok_or_else(|| { + reason_err!( + "rfx::DecodingContext", + "tile quant index {index} out of range for numQuant {}", + quants.len() + ) + }) + }; + tiles .iter() - .map(|t| TileData { - quants: [ - quants[usize::from(t.y_quant_index)].clone(), - quants[usize::from(t.cb_quant_index)].clone(), - quants[usize::from(t.cr_quant_index)].clone(), - ], - data: [t.y_data, t.cb_data, t.cr_data], + .map(|t| { + Ok(TileData { + quants: [ + quant(t.y_quant_index)?, + quant(t.cb_quant_index)?, + quant(t.cr_quant_index)?, + ], + data: [t.y_data, t.cb_data, t.cr_data], + }) }) .collect() } diff --git a/crates/ironrdp-testsuite-core/tests/session/rfx.rs b/crates/ironrdp-testsuite-core/tests/session/rfx.rs index a39fc862bf..cd99286b6b 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; @@ -32,6 +33,54 @@ fn decode_decodes_valid_sequence_of_messages() { assert_eq!(expected, image.data()); } +/// Each quantization index of a [2.2.2.3.4.1] TS_RFX_TILE "specifies an index into the TS_RFX_CODEC_QUANT +/// array provided in the TS_RFX_TILESET message". An index past the end is a decoding error, not a panic. +/// +/// [2.2.2.3.4.1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdprfx/89e669ed-b6dd-4591-a267-73a72bc6d84e +#[rstest] +#[case::y_out_of_range(1, [1, 0, 0])] +#[case::cb_out_of_range(1, [0, 1, 0])] +#[case::cr_out_of_range(1, [0, 0, 1])] +#[case::no_quantization_tables(0, [0, 0, 0])] +fn decode_rejects_tile_quant_index_out_of_range(#[case] quant_count: usize, #[case] quant_indexes: [u8; 3]) { + use ironrdp_pdu::Decode as _; + use ironrdp_pdu::codecs::rfx::{Block, CodecChannel}; + + let destination = InclusiveRectangle { + left: 0, + top: 0, + right: u16::try_from(IMAGE_WIDTH).unwrap() - 1, + bottom: u16::try_from(IMAGE_HEIGHT).unwrap() - 1, + }; + + // Re-encode the valid sequence with only the tileset changed. + 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(); + if let Block::CodecChannel(CodecChannel::TileSet(tile_set)) = &mut block { + tile_set.quants.truncate(quant_count); + let [y, cb, cr] = quant_indexes; + tile_set.tiles[0].y_quant_index = y; + tile_set.tiles[0].cb_quant_index = cb; + tile_set.tiles[0].cr_quant_index = cr; + } + 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(), + ); + + let error = DecodingContext::default() + .decode(&mut image, &destination, &mut ReadCursor::new(&encoded)) + .unwrap_err(); + + assert!(error.to_string().contains("quant"), "unexpected error: {error}"); +} + const ENCODED_MESSAGES: [u8; 2970] = [ /* HEADERS as in 4.2.2 */ 0xc0, 0xcc, 0x0c, 0x00, 0x00, 0x00, 0xca, 0xac, 0xcc, 0xca, 0x00, 0x01, 0xc3, 0xcc, 0x0d, 0x00, 0x00, 0x00, 0x01,