diff --git a/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs b/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs index fff2cb2e..93e00e5b 100644 --- a/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs +++ b/crates/ironrdp-cliprdr/src/pdu/format_data/file_list.rs @@ -1,7 +1,9 @@ use bitflags::bitflags; use ironrdp_pdu::cursor::{ReadCursor, WriteCursor}; -use ironrdp_pdu::utils::{combine_u64, read_string_from_cursor, split_u64, write_string_to_cursor, CharacterSet}; -use ironrdp_pdu::{cast_length, ensure_fixed_part_size, impl_pdu_pod, PduDecode, PduEncode, PduResult}; +use ironrdp_pdu::utils::{combine_u64, decode_string, encode_string, split_u64, CharacterSet}; +use ironrdp_pdu::{cast_length, ensure_fixed_part_size, impl_pdu_pod, write_padding, PduDecode, PduEncode, PduResult}; + +const NAME_LENGTH: usize = 520; bitflags! { /// Represents `flags` field of `CLIPRDR_FILEDESCRIPTOR` structure. @@ -40,12 +42,15 @@ bitflags! { } } -/// Represents `CLIPRDR_FILEDESCRIPTOR` +/// [2.2.5.2.3.1] File Descriptor (CLIPRDR_FILEDESCRIPTOR) +/// +/// [2.2.5.2.3.1]: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpeclip/a765d784-2b39-4b88-9faa-88f8666f9c35 #[derive(Debug, Clone, PartialEq, Eq)] pub struct FileDescriptor { pub attributes: Option, pub last_write_time: Option, pub file_size: Option, + // TODO: Define a new type for "bounded" strings (this one should never be bigger than 260 characters, including the null-terminator) pub name: String, } @@ -59,7 +64,7 @@ impl FileDescriptor { + 16 // reserved + std::mem::size_of::() // last write time + std::mem::size_of::() // size - + 520; // name + + NAME_LENGTH; // name const SIZE: usize = Self::FIXED_PART_SIZE; } @@ -89,12 +94,11 @@ impl PduEncode for FileDescriptor { dst.write_u32(size_hi); dst.write_u32(size_lo); - { - let mut cursor = WriteCursor::new(dst.remaining_mut()); - write_string_to_cursor(&mut cursor, &self.name, CharacterSet::Unicode, true)?; - } + let written = encode_string(dst.remaining_mut(), &self.name, CharacterSet::Unicode, true)?; + dst.advance(written); - dst.advance(520); + // Pad with zeroes, overidding any previously written data + write_padding!(dst, NAME_LENGTH - written); Ok(()) } @@ -136,12 +140,8 @@ impl<'de> PduDecode<'de> for FileDescriptor { None }; - let name = { - let mut cursor = ReadCursor::new(src.remaining()); - read_string_from_cursor(&mut cursor, CharacterSet::Unicode, true)? - }; - - src.advance(520); + let name = decode_string(src.remaining(), CharacterSet::Unicode, true)?; + src.advance(NAME_LENGTH); Ok(Self { attributes, diff --git a/crates/ironrdp-pdu/src/rdp/client_info.rs b/crates/ironrdp-pdu/src/rdp/client_info.rs index 8dcd55c8..3e5de562 100644 --- a/crates/ironrdp-pdu/src/rdp/client_info.rs +++ b/crates/ironrdp-pdu/src/rdp/client_info.rs @@ -70,9 +70,9 @@ impl PduParsing for ClientInfo { let alternate_shell_size = stream.read_u16::()? as usize; let work_dir_size = stream.read_u16::()? as usize; - let domain = utils::read_string(&mut stream, domain_size, character_set, true)?; - let username = utils::read_string(&mut stream, user_name_size, character_set, true)?; - let password = utils::read_string(&mut stream, password_size, character_set, true)?; + let domain = utils::read_string_from_stream(&mut stream, domain_size, character_set, true)?; + let username = utils::read_string_from_stream(&mut stream, user_name_size, character_set, true)?; + let password = utils::read_string_from_stream(&mut stream, password_size, character_set, true)?; let domain = if domain.is_empty() { None } else { Some(domain) }; let credentials = Credentials { @@ -81,8 +81,8 @@ impl PduParsing for ClientInfo { domain, }; - let alternate_shell = utils::read_string(&mut stream, alternate_shell_size, character_set, true)?; - let work_dir = utils::read_string(&mut stream, work_dir_size, character_set, true)?; + let alternate_shell = utils::read_string_from_stream(&mut stream, alternate_shell_size, character_set, true)?; + let work_dir = utils::read_string_from_stream(&mut stream, work_dir_size, character_set, true)?; let extra_info = ExtendedClientInfo::from_buffer(&mut stream, character_set)?; @@ -184,11 +184,11 @@ impl ExtendedClientInfo { // This size includes the length of the mandatory null terminator. let address_size = stream.read_u16::()? as usize; - let address = utils::read_string(&mut stream, address_size, character_set, false)?; + let address = utils::read_string_from_stream(&mut stream, address_size, character_set, false)?; // This size includes the length of the mandatory null terminator. let dir_size = stream.read_u16::()? as usize; - let dir = utils::read_string(&mut stream, dir_size, character_set, false)?; + let dir = utils::read_string_from_stream(&mut stream, dir_size, character_set, false)?; let optional_data = ExtendedClientOptionalInfo::from_buffer(&mut stream)?; @@ -328,11 +328,13 @@ impl PduParsing for TimezoneInfo { fn from_buffer(mut stream: impl io::Read) -> Result { let bias = stream.read_u32::()?; - let standard_name = utils::read_string(&mut stream, TIMEZONE_INFO_NAME_LEN, CharacterSet::Unicode, false)?; + let standard_name = + utils::read_string_from_stream(&mut stream, TIMEZONE_INFO_NAME_LEN, CharacterSet::Unicode, false)?; let standard_date = Option::::from_buffer(&mut stream)?; let standard_bias = stream.read_u32::()?; - let daylight_name = utils::read_string(&mut stream, TIMEZONE_INFO_NAME_LEN, CharacterSet::Unicode, false)?; + let daylight_name = + utils::read_string_from_stream(&mut stream, TIMEZONE_INFO_NAME_LEN, CharacterSet::Unicode, false)?; let daylight_date = Option::::from_buffer(&mut stream)?; let daylight_bias = stream.read_u32::()?; diff --git a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs index 37394010..75990ca1 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/client_new_license_request.rs @@ -147,10 +147,12 @@ impl PduParsing for ClientNewLicenseRequest { stream.read_exact(&mut encrypted_premaster_secret)?; let username_blob_header = BlobHeader::read_from_buffer(BlobType::ClientUserName, &mut stream)?; - let client_username = utils::read_string(&mut stream, username_blob_header.length, CharacterSet::Ansi, false)?; + let client_username = + utils::read_string_from_stream(&mut stream, username_blob_header.length, CharacterSet::Ansi, false)?; let machine_name_blob = BlobHeader::read_from_buffer(BlobType::ClientMachineNameBlob, &mut stream)?; - let client_machine_name = utils::read_string(&mut stream, machine_name_blob.length, CharacterSet::Ansi, false)?; + let client_machine_name = + utils::read_string_from_stream(&mut stream, machine_name_blob.length, CharacterSet::Ansi, false)?; Ok(Self { license_header, diff --git a/crates/ironrdp-pdu/src/rdp/server_license/server_upgrade_license.rs b/crates/ironrdp-pdu/src/rdp/server_license/server_upgrade_license.rs index cfd612b5..4f5b4c7d 100644 --- a/crates/ironrdp-pdu/src/rdp/server_license/server_upgrade_license.rs +++ b/crates/ironrdp-pdu/src/rdp/server_license/server_upgrade_license.rs @@ -105,7 +105,7 @@ impl PduParsing for NewLicenseInformation { let version = stream.read_u32::()?; let scope_len = stream.read_u32::()?; - let scope = utils::read_string( + let scope = utils::read_string_from_stream( &mut stream, scope_len as usize - UTF8_NULL_TERMINATOR_SIZE, CharacterSet::Ansi, @@ -113,7 +113,7 @@ impl PduParsing for NewLicenseInformation { )?; let company_name_len = stream.read_u32::()?; - let company_name = utils::read_string( + let company_name = utils::read_string_from_stream( &mut stream, company_name_len as usize - UTF16_NULL_TERMINATOR_SIZE, CharacterSet::Unicode, @@ -121,7 +121,7 @@ impl PduParsing for NewLicenseInformation { )?; let product_id_len = stream.read_u32::()?; - let product_id = utils::read_string( + let product_id = utils::read_string_from_stream( &mut stream, product_id_len as usize - UTF16_NULL_TERMINATOR_SIZE, CharacterSet::Unicode, diff --git a/crates/ironrdp-pdu/src/rdp/session_info/logon_info.rs b/crates/ironrdp-pdu/src/rdp/session_info/logon_info.rs index bfb63acf..c39b7046 100644 --- a/crates/ironrdp-pdu/src/rdp/session_info/logon_info.rs +++ b/crates/ironrdp-pdu/src/rdp/session_info/logon_info.rs @@ -32,14 +32,16 @@ impl PduParsing for LogonInfoVersion1 { return Err(SessionError::InvalidDomainNameSize); } - let domain_name = utils::read_string(&mut stream, DOMAIN_NAME_SIZE_V1, utils::CharacterSet::Unicode, false)?; + let domain_name = + utils::read_string_from_stream(&mut stream, DOMAIN_NAME_SIZE_V1, utils::CharacterSet::Unicode, false)?; let user_name_size = stream.read_u32::()?; if user_name_size > USER_NAME_SIZE_V1 as u32 { return Err(SessionError::InvalidUserNameSize); } - let user_name = utils::read_string(&mut stream, USER_NAME_SIZE_V1, utils::CharacterSet::Unicode, false)?; + let user_name = + utils::read_string_from_stream(&mut stream, USER_NAME_SIZE_V1, utils::CharacterSet::Unicode, false)?; let session_id = stream.read_u32::()?; @@ -111,13 +113,13 @@ impl PduParsing for LogonInfoVersion2 { let mut padding_buffer = [0; LOGON_INFO_V2_PADDING_SIZE]; stream.read_exact(&mut padding_buffer)?; - let domain_name = utils::read_string( + let domain_name = utils::read_string_from_stream( &mut stream, domain_name_size as usize, utils::CharacterSet::Unicode, false, )?; - let user_name = utils::read_string( + let user_name = utils::read_string_from_stream( &mut stream, user_name_size as usize, utils::CharacterSet::Unicode, diff --git a/crates/ironrdp-pdu/src/rdp/vc/dvc/create.rs b/crates/ironrdp-pdu/src/rdp/vc/dvc/create.rs index c65fda65..ef6368e4 100644 --- a/crates/ironrdp-pdu/src/rdp/vc/dvc/create.rs +++ b/crates/ironrdp-pdu/src/rdp/vc/dvc/create.rs @@ -30,7 +30,7 @@ impl CreateRequestPdu { let channel_id = channel_id_type.read_buffer_according_to_type(&mut stream)?; data_size -= channel_id_type.get_type_size(); - let channel_name = utils::read_string(&mut stream, data_size, utils::CharacterSet::Ansi, false)?; + let channel_name = utils::read_string_from_stream(&mut stream, data_size, utils::CharacterSet::Ansi, false)?; Ok(Self { channel_id_type, diff --git a/crates/ironrdp-pdu/src/utils.rs b/crates/ironrdp-pdu/src/utils.rs index 67ab638b..ea871e33 100644 --- a/crates/ironrdp-pdu/src/utils.rs +++ b/crates/ironrdp-pdu/src/utils.rs @@ -61,8 +61,8 @@ pub fn read_string_from_cursor( cursor .remaining() .chunks_exact(2) - .position(|chunk| chunk[0] == 0 && chunk[1] == 0) - .map(|code_units| code_units + 1) // Read null code point + .position(|chunk| chunk == [0, 0]) + .map(|null_terminator_pos| null_terminator_pos + 1) // Read null code point .unwrap_or(cursor.len() / 2) } else { // UTF16 uses 2 bytes per code unit, so we need to read an even number of bytes @@ -76,7 +76,7 @@ pub fn read_string_from_cursor( .remaining() .iter() .position(|&i| i == 0) - .map(|code_units| code_units + 1) // Read null code point + .map(|null_terminator_pos| null_terminator_pos + 1) // Read null code point .unwrap_or(cursor.len()) } else { // Read all @@ -114,6 +114,10 @@ pub fn read_string_from_cursor( Ok(result.trim_end_matches('\0').into()) } +pub fn decode_string(src: &[u8], character_set: CharacterSet, read_null_terminator: bool) -> PduResult { + read_string_from_cursor(&mut ReadCursor::new(src), character_set, read_null_terminator) +} + pub fn read_multistring_from_cursor( cursor: &mut ReadCursor<'_>, character_set: CharacterSet, @@ -134,18 +138,17 @@ pub fn read_multistring_from_cursor( Ok(strings) } -pub fn write_string_to_cursor( - cursor: &mut WriteCursor<'_>, +pub fn encode_string( + dst: &mut [u8], value: &str, character_set: CharacterSet, write_null_terminator: bool, -) -> PduResult<()> { +) -> PduResult { let (buffer, ctx) = match character_set { CharacterSet::Unicode => { let mut buffer = to_utf16_bytes(value); if write_null_terminator { - buffer.push(0); - buffer.push(0); + buffer.extend_from_slice(&[0, 0]); } (buffer, "Encode string (UTF-16)") } @@ -158,8 +161,22 @@ pub fn write_string_to_cursor( } }; - ensure_size!(ctx: ctx, in: cursor, size: buffer.len()); - cursor.write_slice(&buffer); + let len = buffer.len(); + + ensure_size!(ctx: ctx, in: dst, size: len); + dst[..len].copy_from_slice(&buffer); + + Ok(len) +} + +pub fn write_string_to_cursor( + cursor: &mut WriteCursor<'_>, + value: &str, + character_set: CharacterSet, + write_null_terminator: bool, +) -> PduResult<()> { + let len = encode_string(cursor.remaining_mut(), value, character_set, write_null_terminator)?; + cursor.advance(len); Ok(()) } @@ -207,7 +224,8 @@ pub fn encoded_multistring_len(strings: &[String], character_set: CharacterSet) + if character_set == CharacterSet::Unicode { 2 } else { 1 } } -pub(crate) fn read_string( +// FIXME: legacy +pub(crate) fn read_string_from_stream( mut stream: impl io::Read, size: usize, character_set: CharacterSet, @@ -248,6 +266,7 @@ pub(crate) fn write_string_with_null_terminator( } } +// FIXME: legacy trait pub trait SplitTo { #[must_use] fn split_to(&mut self, n: usize) -> Self;