From c4c14fb1075bf670f683e21dded0cd37809a10fb Mon Sep 17 00:00:00 2001 From: mmorrissette-devolutions Date: Tue, 6 Oct 2026 14:17:41 -0400 Subject: [PATCH] fix(connector): reject out-of-range server desktop sizes The desktop size advertised in the server's Bitmap capability set was adopted as-is, and clients allocate their framebuffer from it. A server announcing 65535x65535 made the client attempt a ~17 GB allocation, and on wasm32 the size computation could overflow. Reject sizes with a zero dimension or a dimension above 32766, the virtual desktop limit servers enforce through ERRINFO_VIRTUALDESKTOPTOOLARGE. This bounds the framebuffer to ~4.3 GB. A valid size can still exceed what wasm32 can allocate, so the web client now probes the framebuffer allocation and fails the session instead of aborting. Native clients keep the infallible allocation and can still abort on hosts that cannot provide a framebuffer of that size. --- .../src/connection_activation.rs | 13 +++++++ .../tests/session/connection_activation.rs | 34 +++++++++++++++++++ crates/ironrdp-web/src/session.rs | 19 +++++++++++ 3 files changed, 66 insertions(+) diff --git a/crates/ironrdp-connector/src/connection_activation.rs b/crates/ironrdp-connector/src/connection_activation.rs index de852dc074..737bb1dd7b 100644 --- a/crates/ironrdp-connector/src/connection_activation.rs +++ b/crates/ironrdp-connector/src/connection_activation.rs @@ -11,6 +11,8 @@ use crate::{ MonotonicInstant, Sequence, State, Written, general_err, reason_err, }; +const MAX_DESKTOP_DIM: u16 = 32766; + /// Represents the Capability Exchange and Connection Finalization phases /// of the connection sequence (section [1.3.1.1]). /// @@ -292,6 +294,17 @@ impl Sequence for ConnectionActivationSequence { height: self.config.desktop_size.height, }); + if !(1..=MAX_DESKTOP_DIM).contains(&desktop_size.width) + || !(1..=MAX_DESKTOP_DIM).contains(&desktop_size.height) + { + return Err(reason_err!( + "ConnectionActivation::CapabilitiesExchange", + "server desktop size {}x{} is outside the supported range", + desktop_size.width, + desktop_size.height + )); + } + let share_id = share_control_ctx.share_id; let client_confirm_active = rdp::headers::ShareControlPdu::ClientConfirmActive( diff --git a/crates/ironrdp-testsuite-core/tests/session/connection_activation.rs b/crates/ironrdp-testsuite-core/tests/session/connection_activation.rs index 95a3d12ffa..0f5a6c002d 100644 --- a/crates/ironrdp-testsuite-core/tests/session/connection_activation.rs +++ b/crates/ironrdp-testsuite-core/tests/session/connection_activation.rs @@ -122,6 +122,40 @@ fn demand_active_static_channel_chunk_size(chunk_size: Option) -> usize { } } +fn step_demand_active_with_desktop_size(width: u16, height: u16) -> ironrdp_connector::ConnectorResult { + let mut demand_active = SERVER_DEMAND_ACTIVE.clone(); + let bitmap = demand_active + .pdu + .capability_sets + .iter_mut() + .find_map(|capability_set| match capability_set { + CapabilitySet::Bitmap(bitmap) => Some(bitmap), + _ => None, + }) + .expect("server demand active should include a bitmap capability"); + bitmap.desktop_width = width; + bitmap.desktop_height = height; + + let mut sequence = ConnectionActivationSequence::new(test_config(), IO_CHANNEL_ID, USER_CHANNEL_ID); + let mut output = WriteBuf::new(); + let frame = encode_server_share_control(ShareControlPdu::ServerDemandActive(demand_active)); + sequence.step(&frame, None, &mut output) +} + +#[test] +fn demand_active_accepts_server_desktop_size_within_range() { + for (width, height) in [(1, 1), (1920, 1080), (32766, 32766)] { + assert!(step_demand_active_with_desktop_size(width, height).is_ok()); + } +} + +#[test] +fn demand_active_rejects_server_desktop_size_out_of_range() { + for (width, height) in [(0, 768), (1024, 0), (32767, 768), (1024, 32767), (u16::MAX, u16::MAX)] { + assert!(step_demand_active_with_desktop_size(width, height).is_err()); + } +} + #[test] fn demand_active_uses_valid_server_static_channel_chunk_size() { for chunk_size in [ diff --git a/crates/ironrdp-web/src/session.rs b/crates/ironrdp-web/src/session.rs index a47bd3d0c0..aa931aceb8 100644 --- a/crates/ironrdp-web/src/session.rs +++ b/crates/ironrdp-web/src/session.rs @@ -670,6 +670,11 @@ impl iron_remote_desktop::Session for Session { let desktop_height = NonZeroU32::new(u32::from(connection_result.desktop_size.height)).context("desktop height is zero")?; + ensure_framebuffer_allocatable( + connection_result.desktop_size.width, + connection_result.desktop_size.height, + )?; + let mut gui = Canvas::new(self.render_canvas.clone(), desktop_width, desktop_height).context("canvas initialization")?; @@ -1073,6 +1078,7 @@ impl iron_remote_desktop::Session for Session { } = connection_activation.connection_activation_state() { debug!("Deactivation-Reactivation Sequence completed"); + ensure_framebuffer_allocatable(desktop_size.width, desktop_size.height)?; image = DecodedImage::new(PixelFormat::RgbA32, desktop_size.width, desktop_size.height); if !active_stage.reactivate( connection_activation.io_channel_id(), @@ -1920,6 +1926,19 @@ where } } +fn ensure_framebuffer_allocatable(width: u16, height: u16) -> anyhow::Result<()> { + let len = usize::from(width) + .checked_mul(usize::from(height)) + .and_then(|pixels| pixels.checked_mul(usize::from(PixelFormat::RgbA32.bytes_per_pixel()))) + .with_context(|| format!("framebuffer size overflows for a {width}x{height} desktop"))?; + + Vec::::new() + .try_reserve_exact(len) + .with_context(|| format!("not enough memory for a {width}x{height} desktop framebuffer"))?; + + Ok(()) +} + #[expect(clippy::as_conversions, clippy::cast_sign_loss, clippy::cast_possible_truncation)] fn f64_to_u16_saturating_cast(value: f64) -> u16 { value as u16