From ef2c3df761805a11ab0a1d6f87b7b1704048b8fe Mon Sep 17 00:00:00 2001 From: Norbert Szetei <59439874+nszetei@users.noreply.github.com> Date: Mon, 11 Mar 2024 14:56:58 +0100 Subject: [PATCH] fix(pdu): fixes three overflows in pointer.rs and one in bitmap.rs (#410) --- crates/ironrdp-fuzzing/src/oracles/mod.rs | 11 +++++++++++ crates/ironrdp-pdu/src/basic_output/bitmap.rs | 8 ++++++++ crates/ironrdp-pdu/src/basic_output/pointer.rs | 17 +++++++++++------ ...sh-638bb782419774c536a3455183fdbf172e5b830b | Bin 0 -> 14 bytes ...sh-661a3fc68cec8096fb13ac76f3b50b85ad4c7ded | Bin 0 -> 22 bytes ...sh-801238c30741729c04951985d7ea1238e930bf80 | Bin 0 -> 14 bytes ...sh-d58836860f9f3b70b3099cfbbed09f763d05eb4d | Bin 0 -> 20 bytes 7 files changed, 30 insertions(+), 6 deletions(-) create mode 100644 crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-638bb782419774c536a3455183fdbf172e5b830b create mode 100644 crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-661a3fc68cec8096fb13ac76f3b50b85ad4c7ded create mode 100644 crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-801238c30741729c04951985d7ea1238e930bf80 create mode 100644 crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-d58836860f9f3b70b3099cfbbed09f763d05eb4d diff --git a/crates/ironrdp-fuzzing/src/oracles/mod.rs b/crates/ironrdp-fuzzing/src/oracles/mod.rs index 143010eb..5d6f1d57 100644 --- a/crates/ironrdp-fuzzing/src/oracles/mod.rs +++ b/crates/ironrdp-fuzzing/src/oracles/mod.rs @@ -45,7 +45,18 @@ pub fn pdu_decode(data: &[u8]) { let _ = decode::(data); let _ = decode::>(data); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::Orders); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::Bitmap); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::Palette); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::Synchronize); let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::SurfaceCommands); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::HiddenPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::DefaultPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::PositionPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::ColorPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::CachedPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::NewPointer); + let _ = fast_path::FastPathUpdate::decode_with_code(data, fast_path::UpdateCode::LargePointer); let _ = decode::>(data); let _ = decode::>(data); diff --git a/crates/ironrdp-pdu/src/basic_output/bitmap.rs b/crates/ironrdp-pdu/src/basic_output/bitmap.rs index a6e7b1fc..c72ad632 100644 --- a/crates/ironrdp-pdu/src/basic_output/bitmap.rs +++ b/crates/ironrdp-pdu/src/basic_output/bitmap.rs @@ -155,6 +155,14 @@ impl<'de> PduDecode<'de> for BitmapData<'de> { let (compressed_data_header, buffer_length) = if compression_flags.contains(Compression::BITMAP_COMPRESSION) && !compression_flags.contains(Compression::NO_BITMAP_COMPRESSION_HDR) { + // Check if encoded_bitmap_data_length is at least CompressedDataHeader::ENCODED_SIZE + if encoded_bitmap_data_length < CompressedDataHeader::ENCODED_SIZE as u16 { + return Err(invalid_message_err!( + "cbCompEncodedBitmapDataLength", + "length is less than CompressedDataHeader::ENCODED_SIZE" + )); + } + let buffer_length = encoded_bitmap_data_length as usize - CompressedDataHeader::ENCODED_SIZE; (Some(CompressedDataHeader::decode(src)?), buffer_length) } else { diff --git a/crates/ironrdp-pdu/src/basic_output/pointer.rs b/crates/ironrdp-pdu/src/basic_output/pointer.rs index d7130c90..35a10016 100644 --- a/crates/ironrdp-pdu/src/basic_output/pointer.rs +++ b/crates/ironrdp-pdu/src/basic_output/pointer.rs @@ -66,6 +66,9 @@ impl ColorPointerAttribute<'_> { const XOR_MASK_SIZE_FIELD: &str = "lengthXorMask"; let check_mask = |mask: &[u8], field: &'static str| { + if pointer_height == 0 { + return Err(invalid_message_err!(field, "pointer height cannot be zero")); + } if large_ptr && (mask.len() > u32::MAX as usize) { return Err(invalid_message_err!(field, "pointer mask is too big for u32 size")); } @@ -130,7 +133,8 @@ impl<'a> PduDecode<'a> for ColorPointerAttribute<'a> { let length_and_mask = src.read_u16(); let length_xor_mask = src.read_u16(); - let expected_masks_size = (length_and_mask + length_xor_mask) as usize; + // Convert to usize during the addition to prevent overflow and match expected type + let expected_masks_size = (length_and_mask as usize) + (length_xor_mask as usize); ensure_size!(in: src, size: expected_masks_size); let xor_mask = src.read_slice(length_xor_mask as usize); @@ -287,14 +291,15 @@ impl<'a> PduDecode<'a> for LargePointerAttribute<'a> { let hot_spot = Point16::decode(src)?; let width = src.read_u16(); let height = src.read_u16(); - let length_and_mask = src.read_u32(); - let length_xor_mask = src.read_u32(); + // Convert to usize to prevent overflow during addition + let length_and_mask = src.read_u32() as usize; + let length_xor_mask = src.read_u32() as usize; - let expected_masks_size = (length_and_mask + length_xor_mask) as usize; + let expected_masks_size = length_and_mask + length_xor_mask; ensure_size!(in: src, size: expected_masks_size); - let xor_mask = src.read_slice(length_xor_mask as usize); - let and_mask = src.read_slice(length_and_mask as usize); + let xor_mask = src.read_slice(length_xor_mask); + let and_mask = src.read_slice(length_and_mask); ColorPointerAttribute::check_masks_alignment(and_mask, xor_mask, height, true)?; diff --git a/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-638bb782419774c536a3455183fdbf172e5b830b b/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-638bb782419774c536a3455183fdbf172e5b830b new file mode 100644 index 0000000000000000000000000000000000000000..0427aa712f4798824764a9195eb6852aac10bba8 GIT binary patch literal 14 VcmZQ#;A3E5U|?ckVEF%^2>=D%0uulL literal 0 HcmV?d00001 diff --git a/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-661a3fc68cec8096fb13ac76f3b50b85ad4c7ded b/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-661a3fc68cec8096fb13ac76f3b50b85ad4c7ded new file mode 100644 index 0000000000000000000000000000000000000000..9255d064a5a08a7bdb21dc87347488e5a0109382 GIT binary patch literal 22 dcmWe<5NA;5)d~NOV literal 0 HcmV?d00001 diff --git a/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-d58836860f9f3b70b3099cfbbed09f763d05eb4d b/crates/ironrdp-testsuite-core/test_data/fuzz_regression/pdu_decode/crash-d58836860f9f3b70b3099cfbbed09f763d05eb4d new file mode 100644 index 0000000000000000000000000000000000000000..3995cec862a619ff13d5faeb4379f902419bf4fd GIT binary patch literal 20 Xcmd-&Ai@9vm-!|68E!FfiY5a9A7leW literal 0 HcmV?d00001