Skip to content

Fix Win7 RDP PDU and bitmap stride handling #1408

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
cxxzhang (cxxzhang) wants to merge 1 commit into
base: master
Choose a base branch
from
+169 −42
Open

Fix Win7 RDP PDU and bitmap stride handling #1408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 131 additions & 41 deletions crates/ironrdp-session/src/fast_path.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::sync::Arc;
use std::{borrow::Cow, sync::Arc};

use ironrdp_bulk::BulkCompressor;
use ironrdp_core::{DecodeErrorKind, ReadCursor, WriteBuf, decode_cursor};
Expand All @@ -21,6 +21,65 @@ use crate::palette::Palette;
use crate::pointer::PointerCache;
use crate::{SessionError, SessionErrorExt as _, SessionResult, custom_err, reason_err, rfx};

fn normalize_bitmap_rows<'a>(
data: &'a [u8],
source_width: usize,
dest_width: usize,
height: usize,
bytes_per_pixel: usize,
source_stride: usize,
) -> Cow<'a, [u8]> {
if data.is_empty() || height == 0 || dest_width == 0 || bytes_per_pixel == 0 {
return Cow::Borrowed(data);
}

let dest_row_bytes = dest_width.saturating_mul(bytes_per_pixel);
let packed_len = dest_row_bytes.saturating_mul(height);
if data.len() == packed_len {
return Cow::Borrowed(data);
}

let tight_source_stride = source_width.saturating_mul(bytes_per_pixel);
let mut stride = source_stride.max(tight_source_stride).max(dest_row_bytes);
if data.len() < stride.saturating_mul(height) {
if data.len() % height != 0 {
return Cow::Borrowed(data);
}

let inferred_stride = data.len() / height;
if inferred_stride < dest_row_bytes {
return Cow::Borrowed(data);
}
stride = inferred_stride;
}

if stride == dest_row_bytes {
return Cow::Borrowed(data);
}

let mut packed = Vec::with_capacity(packed_len);
for row in data.chunks(stride).take(height) {
if row.len() < dest_row_bytes {
return Cow::Borrowed(data);
}
packed.extend_from_slice(&row[..dest_row_bytes]);
}

if packed.len() == packed_len {
debug!(
source_width,
dest_width,
height,
bytes_per_pixel,
source_stride = stride,
"Normalized bitmap rows to destination rectangle width"
);
Cow::Owned(packed)
} else {
Cow::Borrowed(data)
}
}

#[derive(Debug)]
pub enum UpdateKind {
None,
Expand Down Expand Up @@ -211,7 +270,17 @@ impl Processor {
usize::from(update.width),
usize::from(update.height),
) {
Ok(()) => image.apply_rgb24(&buf, &update.rectangle, true)?,
Ok(()) => {
let pixels = normalize_bitmap_rows(
&buf,
usize::from(update.width),
usize::from(update.rectangle.width()),
usize::from(update.height),
3,
usize::from(update.width) * 3,
);
image.apply_rgb24(pixels.as_ref(), &update.rectangle, true)?
}
Err(err) => {
warn!("Invalid RDP6_BITMAP_STREAM: {err}");
update.rectangle.clone()
Expand All @@ -230,11 +299,49 @@ impl Processor {
usize::from(update.height),
usize::from(update.bits_per_pixel),
) {
Ok(RlePixelFormat::Rgb16) => image.apply_rgb16_bitmap(&buf, &update.rectangle)?,
Ok(RlePixelFormat::Rgb15) => image.apply_rgb15_bitmap(&buf, &update.rectangle)?,
Ok(RlePixelFormat::Rgb24) => image.apply_bgr24_bitmap(&buf, &update.rectangle)?,
Ok(RlePixelFormat::Rgb16) => {
let pixels = normalize_bitmap_rows(
&buf,
usize::from(update.width),
usize::from(update.rectangle.width()),
usize::from(update.height),
2,
usize::from(update.width) * 2,
);
image.apply_rgb16_bitmap(pixels.as_ref(), &update.rectangle)?
}
Ok(RlePixelFormat::Rgb15) => {
let pixels = normalize_bitmap_rows(
&buf,
usize::from(update.width),
usize::from(update.rectangle.width()),
usize::from(update.height),
2,
usize::from(update.width) * 2,
);
image.apply_rgb15_bitmap(pixels.as_ref(), &update.rectangle)?
}
Ok(RlePixelFormat::Rgb24) => {
let pixels = normalize_bitmap_rows(
&buf,
usize::from(update.width),
usize::from(update.rectangle.width()),
usize::from(update.height),
3,
usize::from(update.width) * 3,
);
image.apply_bgr24_bitmap(pixels.as_ref(), &update.rectangle)?
}
Ok(RlePixelFormat::Rgb8) => {
image.apply_rgb8_with_palette(&buf, &update.rectangle, self.palette.colors())?
let pixels = normalize_bitmap_rows(
&buf,
usize::from(update.width),
usize::from(update.rectangle.width()),
usize::from(update.height),
1,
usize::from(update.width),
);
image.apply_rgb8_with_palette(pixels.as_ref(), &update.rectangle, self.palette.colors())?
}

Err(e) => {
Expand All @@ -252,45 +359,28 @@ impl Processor {

let bpp = usize::from(update.bits_per_pixel);
let width = usize::from(update.width);
let dest_width = usize::from(update.rectangle.width());
let bytes_per_pixel = bpp.div_ceil(8);
let row_bytes = width * bytes_per_pixel;
let padded_row_bytes = (row_bytes + 3) & !3;
let pixels = normalize_bitmap_rows(
update.bitmap_data,
width,
dest_width,
usize::from(update.height),
bytes_per_pixel,
padded_row_bytes,
);

if padded_row_bytes != row_bytes {
// Strip per-row padding before passing to the bitmap apply functions,
// which expect tightly packed pixel data.
buf.clear();
for row in update.bitmap_data.chunks(padded_row_bytes) {
let end = row_bytes.min(row.len());
buf.extend_from_slice(&row[..end]);
}

match update.bits_per_pixel {
8 => image.apply_rgb8_with_palette(&buf, &update.rectangle, self.palette.colors())?,
15 => image.apply_rgb15_bitmap(&buf, &update.rectangle)?,
16 => image.apply_rgb16_bitmap(&buf, &update.rectangle)?,
24 => image.apply_bgr24_bitmap(&buf, &update.rectangle)?,
32 => image.apply_rgb32_bitmap(&buf, PixelFormat::BgrX32, &update.rectangle)?,
_ => {
warn!("Unsupported uncompressed bitmap depth: {bpp} bpp");
update.rectangle.clone()
}
}
} else {
match update.bits_per_pixel {
8 => image.apply_rgb8_with_palette(
update.bitmap_data,
&update.rectangle,
self.palette.colors(),
)?,
15 => image.apply_rgb15_bitmap(update.bitmap_data, &update.rectangle)?,
16 => image.apply_rgb16_bitmap(update.bitmap_data, &update.rectangle)?,
24 => image.apply_bgr24_bitmap(update.bitmap_data, &update.rectangle)?,
32 => image.apply_rgb32_bitmap(update.bitmap_data, PixelFormat::BgrX32, &update.rectangle)?,
_ => {
warn!("Unsupported uncompressed bitmap depth: {bpp} bpp");
update.rectangle.clone()
}
match update.bits_per_pixel {
8 => image.apply_rgb8_with_palette(pixels.as_ref(), &update.rectangle, self.palette.colors())?,
15 => image.apply_rgb15_bitmap(pixels.as_ref(), &update.rectangle)?,
16 => image.apply_rgb16_bitmap(pixels.as_ref(), &update.rectangle)?,
24 => image.apply_bgr24_bitmap(pixels.as_ref(), &update.rectangle)?,
32 => image.apply_rgb32_bitmap(pixels.as_ref(), PixelFormat::BgrX32, &update.rectangle)?,
_ => {
warn!("Unsupported uncompressed bitmap depth: {bpp} bpp");
update.rectangle.clone()
}
}
};
Expand Down
39 changes: 38 additions & 1 deletion crates/ironrdp-session/src/x224/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ impl Processor {
let channel_id = data_ctx.channel_id;

if channel_id == self.io_channel_id {
self.process_io_channel(data_ctx)
self.process_io_channel_data_indication(data_ctx)
} else if let Some(svc) = self.static_channels.get_by_channel_id_mut(channel_id) {
let response_pdus = svc.process(data_ctx.user_data).map_err(SessionError::pdu)?;
process_svc_messages(response_pdus, channel_id, data_ctx.initiator_id)
Expand All @@ -143,6 +143,43 @@ impl Processor {
}
}

fn process_io_channel_data_indication(
&self,
data_ctx: SendDataIndicationCtx<'_>,
) -> SessionResult<Vec<ProcessorOutput>> {
debug_assert_eq!(data_ctx.channel_id, self.io_channel_id);

let mut outputs = Vec::new();
let mut offset = 0usize;
let data = data_ctx.user_data;
while offset < data.len() {
if offset + 2 > data.len() {
return Err(reason_err!("X224", "truncated Share Control PDU length"));
}

let total_length = usize::from(u16::from_le_bytes([data[offset], data[offset + 1]]));
if total_length == 0 || offset + total_length > data.len() {
if offset == 0 {
return self.process_io_channel(data_ctx);
}
return Err(reason_err!(
"X224",
"invalid concatenated Share Control PDU length: {total_length}"
));
}

let part_ctx = SendDataIndicationCtx {
initiator_id: data_ctx.initiator_id,
channel_id: data_ctx.channel_id,
user_data: &data[offset..offset + total_length],
};
outputs.extend(self.process_io_channel(part_ctx)?);
offset += total_length;
}

Ok(outputs)
}

fn process_io_channel(&self, data_ctx: SendDataIndicationCtx<'_>) -> SessionResult<Vec<ProcessorOutput>> {
debug_assert_eq!(data_ctx.channel_id, self.io_channel_id);

Expand Down