diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71c6be31..40eb62a8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -34,6 +34,15 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Cache + uses: actions/cache@v3 + with: + path: | + ~/.cargo/registry/ + ~/.cargo/git/ + ./target/ + key: ${{ runner.os }}-${{ hashFiles('Cargo.lock') }} + - name: Check clippy run: cargo clippy --workspace -- -D warnings @@ -45,6 +54,15 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Cache + uses: actions/cache@v3 + with: + path: | + ~/.cargo/registry/ + ~/.cargo/git/ + ./ffi/wasm/target/ + key: ${{ runner.os }}-wasm-${{ hashFiles('Cargo.lock') }} + - name: Prepare runner run: sudo apt install wabt @@ -69,5 +87,48 @@ jobs: steps: - uses: actions/checkout@v3 + - name: Cache + uses: actions/cache@v3 + with: + path: | + ~/.cargo/registry/ + ~/.cargo/git/ + ./target/ + key: ${{ runner.os }}-${{ hashFiles('Cargo.lock') }} + - name: Test [${{ matrix.os }}] run: cargo test --workspace + + fuzz: + name: Fuzzing + runs-on: ubuntu-20.04 + needs: formatting + + steps: + - uses: actions/checkout@v3 + + - name: Fuzz build cache + uses: actions/cache@v3 + with: + path: | + ./fuzz/target/ + ./artifacts/ + key: ${{ runner.os }}-fuzz-${{ hashFiles('./fuzz/Cargo.lock') }} + + - name: Fuzz corpus cache + uses: actions/cache@v3 + with: + path: | + ./fuzz/corpus/ + key: fuzz-corpus-cache + + - name: Prepare runner + run: | + cd ./fuzz/ + cargo install cargo-fuzz + rustup install nightly --profile=minimal + + - name: Fuzz + run: | + rustup run nightly cargo fuzz run pdu_decoding -- -max_total_time=3s + rustup run nightly cargo fuzz run rle_decompression -- -max_total_time=3s diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 00000000..80e257d5 --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,3 @@ +/target +/artifacts +/corpus diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 00000000..d90b76a1 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "ironrdp-fuzz" +version = "0.0.0" +publish = false +edition = "2021" + +[package.metadata] +cargo-fuzz = true + +[workspace] +members = ["."] + +[profile.release] +debug = 1 + +[dependencies] +ironrdp-core = { path = "../ironrdp-core" } +ironrdp-graphics = { path = "../ironrdp-graphics" } +libfuzzer-sys = "0.4" +arbitrary = { version = "1", features = ["derive"] } +bytes = "1.4.0" + +[[bin]] +name = "pdu_decoding" +path = "fuzz_targets/pdu_decoding.rs" + +[[bin]] +name = "rle_decompression" +path = "fuzz_targets/rle_decompression.rs" diff --git a/ironrdp-core/fuzz/fuzz_targets/fuzz_pdu.rs b/fuzz/fuzz_targets/pdu_decoding.rs similarity index 91% rename from ironrdp-core/fuzz/fuzz_targets/fuzz_pdu.rs rename to fuzz/fuzz_targets/pdu_decoding.rs index 03f9d336..f61245d6 100644 --- a/ironrdp-core/fuzz/fuzz_targets/fuzz_pdu.rs +++ b/fuzz/fuzz_targets/pdu_decoding.rs @@ -1,10 +1,8 @@ #![no_main] -#[macro_use] -extern crate libfuzzer_sys; -extern crate ironrdp; -use ironrdp::rdp::*; -use ironrdp::*; +use ironrdp_core::rdp::*; +use ironrdp_core::*; +use libfuzzer_sys::fuzz_target; fuzz_target!(|data: &[u8]| { let _ = Request::from_buffer(data); @@ -35,10 +33,7 @@ fuzz_target!(|data: &[u8]| { let _ = fast_path::FastPathHeader::from_buffer(data); let _ = fast_path::FastPathUpdatePdu::from_buffer(data); - let _ = fast_path::FastPathUpdate::from_buffer_with_code( - data, - fast_path::UpdateCode::SurfaceCommands, - ); + let _ = fast_path::FastPathUpdate::from_buffer_with_code(data, fast_path::UpdateCode::SurfaceCommands); let _ = surface_commands::SurfaceCommand::from_buffer(data); let _ = surface_commands::SurfaceBitsPdu::from_buffer(data); diff --git a/fuzz/fuzz_targets/rle_decompression.rs b/fuzz/fuzz_targets/rle_decompression.rs new file mode 100644 index 00000000..e8c7f5e7 --- /dev/null +++ b/fuzz/fuzz_targets/rle_decompression.rs @@ -0,0 +1,20 @@ +#![no_main] + +use bytes::BytesMut; +use libfuzzer_sys::fuzz_target; + +#[derive(arbitrary::Arbitrary, Debug)] +struct Input<'a> { + src: &'a [u8], + width: u8, + height: u8, +} + +fuzz_target!(|input: Input<'_>| { + let mut out = BytesMut::new(); + + let _ = ironrdp_graphics::rle::decompress_24_bpp(input.src, &mut out, input.width, input.height); + let _ = ironrdp_graphics::rle::decompress_16_bpp(input.src, &mut out, input.width, input.height); + let _ = ironrdp_graphics::rle::decompress_15_bpp(input.src, &mut out, input.width, input.height); + let _ = ironrdp_graphics::rle::decompress_8_bpp(input.src, &mut out, input.width, input.height); +}); diff --git a/fuzz/rust-toolchain.toml b/fuzz/rust-toolchain.toml new file mode 100644 index 00000000..67d9a53d --- /dev/null +++ b/fuzz/rust-toolchain.toml @@ -0,0 +1,3 @@ +[toolchain] +channel = "nightly" +profile = "minimal" diff --git a/ironrdp-core/fuzz/.gitignore b/ironrdp-core/fuzz/.gitignore deleted file mode 100644 index 572e03bd..00000000 --- a/ironrdp-core/fuzz/.gitignore +++ /dev/null @@ -1,4 +0,0 @@ - -target -corpus -artifacts diff --git a/ironrdp-core/fuzz/Cargo.toml b/ironrdp-core/fuzz/Cargo.toml deleted file mode 100644 index ffa4076f..00000000 --- a/ironrdp-core/fuzz/Cargo.toml +++ /dev/null @@ -1,20 +0,0 @@ -[package] -name = "ironrdp-fuzz" -version = "0.0.1" -authors = ["Automatically generated"] -publish = false - -[[bin]] -name = "fuzz_pdu" -path = "fuzz_targets/fuzz_pdu.rs" - -[package.metadata] -cargo-fuzz = true - -# Prevent this from interfering with workspaces -[workspace] -members = ["."] - -[dependencies] -ironrdp = { path = ".." } -libfuzzer-sys = { git = "https://github.com/rust-fuzz/libfuzzer-sys.git" } diff --git a/ironrdp-core/src/connection_initiation.rs b/ironrdp-core/src/connection_initiation.rs index c22ba309..634ae380 100644 --- a/ironrdp-core/src/connection_initiation.rs +++ b/ironrdp-core/src/connection_initiation.rs @@ -102,6 +102,8 @@ pub enum NegotiationError { ResponseFailure(FailureCode), #[error("Invalid tpkt header version")] TpktVersionError, + #[error("Not enough bytes")] + NotEnoughBytes, } impl From for io::Error { @@ -138,6 +140,10 @@ impl PduParsing for Request { read_and_check_class(&mut stream, 0)?; + if tpkt.length < TPDU_REQUEST_LENGTH { + return Err(NegotiationError::NotEnoughBytes); + } + let mut buffer = vec![0u8; tpkt.length - TPDU_REQUEST_LENGTH]; stream.read_exact(buffer.as_mut_slice())?; diff --git a/ironrdp-core/src/rdp.rs b/ironrdp-core/src/rdp.rs index 2f029e3b..35f906f4 100644 --- a/ironrdp-core/src/rdp.rs +++ b/ironrdp-core/src/rdp.rs @@ -108,6 +108,8 @@ pub enum RdpError { ServerSetErrorInfoError(#[from] ServerSetErrorInfoError), #[error("Input event PDU error")] InputEventError(#[from] InputEventError), + #[error("Not enough bytes")] + NotEnoughBytes, } impl From for io::Error { diff --git a/ironrdp-core/src/rdp/headers.rs b/ironrdp-core/src/rdp/headers.rs index f2729ca9..79070c8d 100644 --- a/ironrdp-core/src/rdp/headers.rs +++ b/ironrdp-core/src/rdp/headers.rs @@ -89,12 +89,18 @@ impl PduParsing for ShareControlHeader { pdu_source, share_id, }; + if pdu_type == ShareControlPduType::DataPdu { // Some windows version have an issue where PDU // there is some padding not part of the inner unit. // Consume that data let header_length = header.buffer_length(); + if header_length != total_length { + if total_length < header_length { + return Err(RdpError::NotEnoughBytes); + } + let padding = total_length - header_length; let mut data = vec![0u8; padding]; stream.read_exact(data.as_mut())?; @@ -103,6 +109,7 @@ impl PduParsing for ShareControlHeader { Ok(header) } + fn to_buffer(&self, mut stream: impl io::Write) -> Result<(), Self::Error> { let pdu_type_with_version = PROTOCOL_VERSION | self.share_control_pdu.share_header_type().to_u16().unwrap(); @@ -114,6 +121,7 @@ impl PduParsing for ShareControlHeader { self.share_control_pdu.to_buffer(&mut stream) } + fn buffer_length(&self) -> usize { SHARE_CONTROL_HEADER_SIZE + self.share_control_pdu.buffer_length() } diff --git a/ironrdp-core/src/rdp/server_license.rs b/ironrdp-core/src/rdp/server_license.rs index 6ee2ab21..78f409b5 100644 --- a/ironrdp-core/src/rdp/server_license.rs +++ b/ironrdp-core/src/rdp/server_license.rs @@ -228,6 +228,8 @@ pub enum ServerLicenseError { InvalidScopeCount(u32), #[error("Received invalid sertificate length: {0}")] InvalidCertificateLength(u32), + #[error("Blob too small")] + BlobTooSmall, } pub struct BlobHeader { diff --git a/ironrdp-core/src/rdp/server_license/server_license_request.rs b/ironrdp-core/src/rdp/server_license/server_license_request.rs index 2e11feb5..4092251e 100644 --- a/ironrdp-core/src/rdp/server_license/server_license_request.rs +++ b/ironrdp-core/src/rdp/server_license/server_license_request.rs @@ -231,6 +231,9 @@ impl PduParsing for Scope { fn from_buffer(mut stream: impl io::Read) -> Result { let blob_header = BlobHeader::read_from_buffer(BlobType::Scope, &mut stream)?; + if blob_header.length < UTF8_NULL_TERMINATOR_SIZE { + return Err(ServerLicenseError::BlobTooSmall); + } let mut blob_data = vec![0u8; blob_header.length]; stream.read_exact(&mut blob_data)?; blob_data.resize(blob_data.len() - UTF8_NULL_TERMINATOR_SIZE, 0); diff --git a/ironrdp-graphics/src/rle.rs b/ironrdp-graphics/src/rle.rs index f4317853..eefc1266 100644 --- a/ironrdp-graphics/src/rle.rs +++ b/ironrdp-graphics/src/rle.rs @@ -14,7 +14,13 @@ use bytes::BytesMut; use core::fmt; use std::ops::BitXor; -// TODO: error handling +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum RlePixelFormat { + Rgb24, + Rgb16, + Rgb15, + Rgb8, +} /// Decompress an RLE compressed bitmap. /// @@ -29,13 +35,13 @@ pub fn decompress( width: impl Into, height: impl Into, bpp: impl Into, -) { +) -> Result { match bpp.into() { Mode24Bpp::BPP => decompress_24_bpp(src, dst, width, height), Mode16Bpp::BPP => decompress_16_bpp(src, dst, width, height), Mode15Bpp::BPP => decompress_15_bpp(src, dst, width, height), Mode8Bpp::BPP => decompress_8_bpp(src, dst, width, height), - _ => todo!("return error"), + invalid => Err(RleError::InvalidBpp { bpp: invalid }), } } @@ -45,7 +51,12 @@ pub fn decompress( /// `dst`: destination buffer /// `width`: decompressed bitmap width /// `height`: decompressed bitmap height -pub fn decompress_24_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into, height: impl Into) { +pub fn decompress_24_bpp( + src: &[u8], + dst: &mut BytesMut, + width: impl Into, + height: impl Into, +) -> Result { decompress_helper::(src, dst, width.into(), height.into()) } @@ -55,7 +66,12 @@ pub fn decompress_24_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into /// `dst`: destination buffer /// `width`: decompressed bitmap width /// `height`: decompressed bitmap height -pub fn decompress_16_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into, height: impl Into) { +pub fn decompress_16_bpp( + src: &[u8], + dst: &mut BytesMut, + width: impl Into, + height: impl Into, +) -> Result { decompress_helper::(src, dst, width.into(), height.into()) } @@ -65,7 +81,12 @@ pub fn decompress_16_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into /// `dst`: destination buffer /// `width`: decompressed bitmap width /// `height`: decompressed bitmap height -pub fn decompress_15_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into, height: impl Into) { +pub fn decompress_15_bpp( + src: &[u8], + dst: &mut BytesMut, + width: impl Into, + height: impl Into, +) -> Result { decompress_helper::(src, dst, width.into(), height.into()) } @@ -75,14 +96,91 @@ pub fn decompress_15_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into /// `dst`: destination buffer /// `width`: decompressed bitmap width /// `height`: decompressed bitmap height -pub fn decompress_8_bpp(src: &[u8], dst: &mut BytesMut, width: impl Into, height: impl Into) { +pub fn decompress_8_bpp( + src: &[u8], + dst: &mut BytesMut, + width: impl Into, + height: impl Into, +) -> Result { decompress_helper::(src, dst, width.into(), height.into()) } -fn decompress_helper(src: &[u8], dst: &mut BytesMut, width: usize, height: usize) { +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RleError { + InvalidBpp { + bpp: usize, + }, + BadOrderCode, + NotEnoughBytes { + expected: usize, + actual: usize, + }, + InvalidImageSize { + maximum_additional: usize, + required_additional: usize, + }, + EmptyImage, + UnexpectedZeroLength, +} + +impl fmt::Display for RleError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + RleError::InvalidBpp { bpp } => write!(f, "invalid bytes per pixel: {bpp}"), + RleError::BadOrderCode => write!(f, "bad RLE order code"), + RleError::NotEnoughBytes { expected, actual } => { + write!(f, "not enough bytes: expected {expected} bytes, but got {actual}") + } + RleError::InvalidImageSize { + maximum_additional, + required_additional, + } => { + write!( + f, + "invalid image size advertised: output buffer can only receive at most {maximum_additional} additional bytes, but {required_additional} bytes are required" + ) + } + RleError::EmptyImage => write!(f, "height or width is zero"), + RleError::UnexpectedZeroLength => write!(f, "unexpected zero-length"), + } + } +} + +fn decompress_helper( + src: &[u8], + dst: &mut BytesMut, + width: usize, + height: usize, +) -> Result { + if width == 0 || height == 0 { + return Err(RleError::EmptyImage); + } + let row_delta = Mode::COLOR_DEPTH * width; dst.resize(row_delta * height, 0); - decompress_impl::(src, dst, row_delta); + decompress_impl::(src, dst, row_delta)?; + + Ok(Mode::PIXEL_FORMAT) +} + +macro_rules! ensure_size { + (from: $buf:ident, size: $expected:expr) => {{ + let actual = $buf.remaining_len(); + let expected = $expected; + if expected > actual { + return Err(RleError::NotEnoughBytes { expected, actual }); + } + }}; + (into: $buf:ident, size: $required_additional:expr) => {{ + let maximum_additional = $buf.remaining_len(); + let required_additional = $required_additional; + if required_additional > maximum_additional { + return Err(RleError::InvalidImageSize { + maximum_additional, + required_additional, + }); + } + }}; } /// RLE decompression implementation @@ -90,9 +188,7 @@ fn decompress_helper(src: &[u8], dst: &mut BytesMut, width: usi /// `src`: source buffer containing compressed bitmap /// `dst`: destination buffer /// `row_delta`: scanline length in bytes -fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize) { - // FIXME: some bounds checks should be inserted - +fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize) -> Result<(), RleError> { let mut src = Buf::new(src); let mut dst = BufMut::new(dst); @@ -107,16 +203,20 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize insert_fg_pel = false; } + ensure_size!(from: src, size: 1); + let header = src.read_u8(); // Extract the compression order code ID from the compression order header. let code = Code::decode(header); // Extract run length - let run_length = code.extract_run_length(header, &mut src); + let run_length = code.extract_run_length(header, &mut src)?; // Handle Background Run Orders. if code == Code::REGULAR_BG_RUN || code == Code::MEGA_MEGA_BG_RUN { + ensure_size!(into: dst, size: run_length * Mode::COLOR_DEPTH); + if is_first_line { let num_iterations = if insert_fg_pel { Mode::write_pixel(&mut dst, fg_pel); @@ -161,14 +261,20 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize { // Handle Foreground Run Orders. + ensure_size!(from: src, size: Mode::COLOR_DEPTH); + if code == Code::LITE_SET_FG_FG_RUN || code == Code::MEGA_MEGA_SET_FG_RUN { fg_pel = Mode::read_pixel(&mut src); } - for _ in 0..run_length { - if is_first_line { + ensure_size!(into: dst, size: run_length * Mode::COLOR_DEPTH); + + if is_first_line { + for _ in 0..run_length { Mode::write_pixel(&mut dst, fg_pel); - } else { + } + } else { + for _ in 0..run_length { let pixel_above = dst.read_pixel_above::(row_delta); let xored = pixel_above ^ fg_pel; Mode::write_pixel(&mut dst, xored); @@ -177,9 +283,13 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize } else if code == Code::LITE_DITHERED_RUN || code == Code::MEGA_MEGA_DITHERED_RUN { // Handle Dithered Run Orders. + ensure_size!(from: src, size: 2 * Mode::COLOR_DEPTH); + let pixel_a = Mode::read_pixel(&mut src); let pixel_b = Mode::read_pixel(&mut src); + ensure_size!(into: dst, size: run_length * 2 * Mode::COLOR_DEPTH); + for _ in 0..run_length { Mode::write_pixel(&mut dst, pixel_a); Mode::write_pixel(&mut dst, pixel_b); @@ -187,8 +297,12 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize } else if code == Code::REGULAR_COLOR_RUN || code == Code::MEGA_MEGA_COLOR_RUN { // Handle Color Run Orders. + ensure_size!(from: src, size: Mode::COLOR_DEPTH); + let pixel = Mode::read_pixel(&mut src); + ensure_size!(into: dst, size: run_length * Mode::COLOR_DEPTH); + for _ in 0..run_length { Mode::write_pixel(&mut dst, pixel); } @@ -200,6 +314,7 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize // Handle Foreground/Background Image Orders. if code == Code::LITE_SET_FG_FGBG_IMAGE || code == Code::MEGA_MEGA_SET_FGBG_IMAGE { + ensure_size!(from: src, size: Mode::COLOR_DEPTH); fg_pel = Mode::read_pixel(&mut src); } @@ -208,12 +323,13 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize while number_to_read > 0 { let c_bits = std::cmp::min(8, number_to_read); + ensure_size!(from: src, size: 1); let bitmask = src.read_u8(); if is_first_line { - write_first_line_fg_bg_image::(&mut dst, bitmask, fg_pel, c_bits); + write_first_line_fg_bg_image::(&mut dst, bitmask, fg_pel, c_bits)?; } else { - write_fg_bg_image::(&mut dst, row_delta, bitmask, fg_pel, c_bits); + write_fg_bg_image::(&mut dst, row_delta, bitmask, fg_pel, c_bits)?; } number_to_read -= c_bits; @@ -223,6 +339,9 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize let byte_count = run_length * Mode::COLOR_DEPTH; + ensure_size!(from: src, size: byte_count); + ensure_size!(into: dst, size: byte_count); + for _ in 0..byte_count { dst.write_u8(src.read_u8()); } @@ -232,9 +351,9 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize const MASK_SPECIAL_FG_BG_1: u8 = 0x03; if is_first_line { - write_first_line_fg_bg_image::(&mut dst, MASK_SPECIAL_FG_BG_1, fg_pel, 8); + write_first_line_fg_bg_image::(&mut dst, MASK_SPECIAL_FG_BG_1, fg_pel, 8)?; } else { - write_fg_bg_image::(&mut dst, row_delta, MASK_SPECIAL_FG_BG_1, fg_pel, 8); + write_fg_bg_image::(&mut dst, row_delta, MASK_SPECIAL_FG_BG_1, fg_pel, 8)?; } } else if code == Code::SPECIAL_FGBG_2 { // Handle Special Order 2. @@ -242,22 +361,28 @@ fn decompress_impl(src: &[u8], dst: &mut [u8], row_delta: usize const MASK_SPECIAL_FG_BG_2: u8 = 0x05; if is_first_line { - write_first_line_fg_bg_image::(&mut dst, MASK_SPECIAL_FG_BG_2, fg_pel, 8); + write_first_line_fg_bg_image::(&mut dst, MASK_SPECIAL_FG_BG_2, fg_pel, 8)?; } else { - write_fg_bg_image::(&mut dst, row_delta, MASK_SPECIAL_FG_BG_2, fg_pel, 8); + write_fg_bg_image::(&mut dst, row_delta, MASK_SPECIAL_FG_BG_2, fg_pel, 8)?; } } else if code == Code::SPECIAL_WHITE { // Handle White Order. + ensure_size!(into: dst, size: Mode::COLOR_DEPTH); + Mode::write_pixel(&mut dst, Mode::WHITE_PIXEL); } else if code == Code::SPECIAL_BLACK { // Handle Black Order. + ensure_size!(into: dst, size: Mode::COLOR_DEPTH); + Mode::write_pixel(&mut dst, Mode::BLACK_PIXEL); } else { - todo!("return unknown order error"); + return Err(RleError::BadOrderCode); } } + + Ok(()) } #[derive(Clone, Copy, PartialEq, Eq)] @@ -338,11 +463,11 @@ impl Code { } /// Extract the run length of a compression order. - pub fn extract_run_length(self, header: u8, src: &mut Buf) -> usize { + pub fn extract_run_length(self, header: u8, src: &mut Buf) -> Result { match self { - Self::REGULAR_FGBG_IMAGE => extract_run_length_regular_fg_bg(header, src), + Self::REGULAR_FGBG_IMAGE => extract_run_length_fg_bg(header, MASK_REGULAR_RUN_LENGTH, src), - Self::LITE_SET_FG_FGBG_IMAGE => extract_run_length_lite_fg_bg(header, src), + Self::LITE_SET_FG_FGBG_IMAGE => extract_run_length_fg_bg(header, MASK_LITE_RUN_LENGTH, src), Self::REGULAR_BG_RUN | Self::REGULAR_FG_RUN | Self::REGULAR_COLOR_RUN | Self::REGULAR_COLOR_IMAGE => { extract_run_length_regular(header, src) @@ -359,9 +484,9 @@ impl Code { | Self::MEGA_MEGA_SET_FGBG_IMAGE | Self::MEGA_MEGA_COLOR_IMAGE => extract_run_length_mega_mega(src), - Self::SPECIAL_FGBG_1 | Self::SPECIAL_FGBG_2 | Self::SPECIAL_WHITE | Self::SPECIAL_BLACK => 0, + Self::SPECIAL_FGBG_1 | Self::SPECIAL_FGBG_2 | Self::SPECIAL_WHITE | Self::SPECIAL_BLACK => Ok(0), - _ => 0, + _ => Ok(0), } } } @@ -369,41 +494,50 @@ impl Code { const MASK_REGULAR_RUN_LENGTH: u8 = 0x1F; const MASK_LITE_RUN_LENGTH: u8 = 0x0F; -/// Extract the run length of a Regular-Form Foreground/Background Image Order -fn extract_run_length_regular_fg_bg(header: u8, src: &mut Buf) -> usize { - match header & MASK_REGULAR_RUN_LENGTH { - 0 => usize::from(src.read_u8()) + 1, - run_length => usize::from(run_length) * 8, - } -} - -/// Extract the run length of a Lite-Form Foreground/Background Image Order. -fn extract_run_length_lite_fg_bg(header: u8, src: &mut Buf) -> usize { - match header & MASK_LITE_RUN_LENGTH { - 0 => usize::from(src.read_u8()) + 1, - run_length => usize::from(run_length) * 8, +/// Extract the run length of a Foreground/Background Image Order. +fn extract_run_length_fg_bg(header: u8, length_mask: u8, src: &mut Buf) -> Result { + match header & length_mask { + 0 => { + ensure_size!(from: src, size: 1); + Ok(usize::from(src.read_u8()) + 1) + } + run_length => Ok(usize::from(run_length) * 8), } } /// Extract the run length of a regular-form compression order. -fn extract_run_length_regular(header: u8, src: &mut Buf) -> usize { +fn extract_run_length_regular(header: u8, src: &mut Buf) -> Result { match header & MASK_REGULAR_RUN_LENGTH { - // An extended (MEGA) run. - 0 => usize::from(src.read_u8()) + 32, - run_length => usize::from(run_length), + 0 => { + // An extended (MEGA) run. + ensure_size!(from: src, size: 1); + Ok(usize::from(src.read_u8()) + 32) + } + run_length => Ok(usize::from(run_length)), } } -fn extract_run_length_lite(header: u8, src: &mut Buf) -> usize { +fn extract_run_length_lite(header: u8, src: &mut Buf) -> Result { match header & MASK_LITE_RUN_LENGTH { - // An extended (MEGA) run. - 0 => usize::from(src.read_u8()) + 16, - run_length => usize::from(run_length), + 0 => { + // An extended (MEGA) run. + ensure_size!(from: src, size: 1); + Ok(usize::from(src.read_u8()) + 16) + } + run_length => Ok(usize::from(run_length)), } } -fn extract_run_length_mega_mega(src: &mut Buf) -> usize { - usize::from(src.read_u16()) +fn extract_run_length_mega_mega(src: &mut Buf) -> Result { + ensure_size!(from: src, size: 2); + + let run_length = usize::from(src.read_u16()); + + if run_length == 0 { + Err(RleError::UnexpectedZeroLength) + } else { + Ok(run_length) + } } struct Buf<'a> { @@ -416,6 +550,10 @@ impl<'a> Buf<'a> { Self { inner: bytes, pos: 0 } } + fn remaining_len(&self) -> usize { + self.inner.len() - self.pos + } + fn read(&mut self) -> [u8; N] { let bytes = &self.inner[self.pos..self.pos + N]; self.pos += N; @@ -457,6 +595,10 @@ impl<'a> BufMut<'a> { Self { inner: bytes, pos: 0 } } + fn remaining_len(&self) -> usize { + self.inner.len() - self.pos + } + fn write(&mut self, bytes: &[u8]) { self.inner[self.pos..self.pos + bytes.len()].copy_from_slice(bytes); self.pos += bytes.len(); @@ -493,6 +635,9 @@ trait DepthMode { /// Bits per pixel const BPP: usize; + /// Pixel format for this depth mode + const PIXEL_FORMAT: RlePixelFormat; + /// The black pixel value const BLACK_PIXEL: Self::Pixel; @@ -515,6 +660,8 @@ impl DepthMode for Mode8Bpp { const BPP: usize = 8; + const PIXEL_FORMAT: RlePixelFormat = RlePixelFormat::Rgb8; + const BLACK_PIXEL: Self::Pixel = 0x00; const WHITE_PIXEL: Self::Pixel = 0xFF; @@ -537,6 +684,8 @@ impl DepthMode for Mode15Bpp { const BPP: usize = 15; + const PIXEL_FORMAT: RlePixelFormat = RlePixelFormat::Rgb15; + const BLACK_PIXEL: Self::Pixel = 0x0000; // 5 bits per RGB component: @@ -561,6 +710,8 @@ impl DepthMode for Mode16Bpp { const BPP: usize = 16; + const PIXEL_FORMAT: RlePixelFormat = RlePixelFormat::Rgb16; + const BLACK_PIXEL: Self::Pixel = 0x0000; // 5 bits for red, 6 bits for green, 5 bits for green: @@ -585,6 +736,8 @@ impl DepthMode for Mode24Bpp { const BPP: usize = 24; + const PIXEL_FORMAT: RlePixelFormat = RlePixelFormat::Rgb24; + const BLACK_PIXEL: Self::Pixel = 0x000000; // 8 bits per RGB component: @@ -607,7 +760,9 @@ fn write_fg_bg_image( bitmask: u8, fg_pel: Mode::Pixel, mut c_bits: usize, -) { +) -> Result<(), RleError> { + ensure_size!(into: dst, size: c_bits * Mode::COLOR_DEPTH); + let mut mask = 0x01; repeat::<8>(|| { @@ -623,7 +778,9 @@ fn write_fg_bg_image( mask <<= 1; c_bits == 0 - }) + }); + + Ok(()) } /// Writes a foreground/background image to a destination buffer @@ -632,7 +789,9 @@ fn write_first_line_fg_bg_image( bitmask: u8, fg_pel: Mode::Pixel, mut c_bits: usize, -) { +) -> Result<(), RleError> { + ensure_size!(into: dst, size: c_bits * Mode::COLOR_DEPTH); + let mut mask = 0x01; repeat::<8>(|| { @@ -646,7 +805,9 @@ fn write_first_line_fg_bg_image( mask <<= 1; c_bits == 0 - }) + }); + + Ok(()) } fn repeat(mut op: impl FnMut() -> bool) { diff --git a/ironrdp-graphics/tests/rle.rs b/ironrdp-graphics/tests/rle.rs index 8595f39d..592559a3 100644 --- a/ironrdp-graphics/tests/rle.rs +++ b/ironrdp-graphics/tests/rle.rs @@ -37,7 +37,7 @@ fn decompress_bpp_16(#[case] src: &[u8]) { // IronRDP let mut ironrdp_out = bytes::BytesMut::new(); - ironrdp_graphics::rle::decompress_16_bpp(src, &mut ironrdp_out, WIDTH, HEIGHT); + ironrdp_graphics::rle::decompress_16_bpp(src, &mut ironrdp_out, WIDTH, HEIGHT).expect("ironrdp decompress"); ironrdp_out.freeze() }; diff --git a/ironrdp-session/src/active_session/fast_path.rs b/ironrdp-session/src/active_session/fast_path.rs index 57b9b7be..01576cf2 100644 --- a/ironrdp-session/src/active_session/fast_path.rs +++ b/ironrdp-session/src/active_session/fast_path.rs @@ -8,6 +8,7 @@ use ironrdp_core::fast_path::{ use ironrdp_core::geometry::Rectangle; use ironrdp_core::surface_commands::{FrameAction, FrameMarkerPdu, SurfaceCommand}; use ironrdp_core::{PduBufferParsing, ShareDataPdu}; +use ironrdp_graphics::rle::RlePixelFormat; use num_traits::FromPrimitive; use super::codecs::rfx; @@ -85,16 +86,24 @@ impl Processor { update.bits_per_pixel ); - ironrdp_graphics::rle::decompress( + match ironrdp_graphics::rle::decompress( update.bitmap_data, &mut buf, update.width, update.height, update.bits_per_pixel, - ); + ) { + Ok(RlePixelFormat::Rgb16) => { + image.apply_rgb16_bitmap(&buf, &update.rectangle); + } - // TODO: support other pixel formats… - image.apply_rgb16_bitmap(&buf, &update.rectangle); + // TODO: support other pixel formats… + Ok(format @ (RlePixelFormat::Rgb8 | RlePixelFormat::Rgb15 | RlePixelFormat::Rgb24)) => { + warn!("Received RLE-compressed bitmap with unsupported color depth: {format:?}"); + } + + Err(e) => warn!("Invalid RLE-compressed bitmap: {e}"), + } } } else { // Uncompressed bitmap data is formatted as a bottom-up, left-to-right series of @@ -102,8 +111,11 @@ impl Processor { // four bytes (including up to three bytes of padding, as necessary). trace!("Uncompressed raw bitmap"); - // TODO: support other pixel formats… - image.apply_rgb16_bitmap(update.bitmap_data, &update.rectangle); + match update.bits_per_pixel { + 16 => image.apply_rgb16_bitmap(update.bitmap_data, &update.rectangle), + // TODO: support other pixel formats… + unsupported => warn!("Invalid raw bitmap with {unsupported} bytes per pixels"), + } } match update_rectangle {