fix(graphics): harden RLE implementation (#86)

Mostly added bound checks in case an invalid RLE-compressed bitmap is
received. Implementation have been fuzzed for a few hours in order to
find blind spots.

.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

fuzz/.gitignore

@@ -0,0 +1,3 @@
+/target
+/artifacts
+/corpus

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"

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);

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

fuzz/rust-toolchain.toml

@@ -0,0 +1,3 @@
+[toolchain]
+channel = "nightly"
+profile = "minimal"

ironrdp-core/fuzz/.gitignore

@@ -1,4 +0,0 @@
-
-target
-corpus
-artifacts

ironrdp-core/fuzz/Cargo.toml

@@ -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" }

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<NegotiationError> 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())?;

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<RdpError> for io::Error {

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()
     }

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 {

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<Self, Self::Error> {
         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);

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()
     };

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 {