fix(cliprdr): ensure string padding is zeroed (#239)

This commit is contained in:
Benoît Cortier
2023-10-30 16:38:48 +02:00
committed by GitHub
parent 7d648ef56e
commit 9e287b6cf7
7 changed files with 70 additions and 45 deletions
@@ -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<ClipboardFileAttributes>,
pub last_write_time: Option<u64>,
pub file_size: Option<u64>,
// 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::<u64>() // last write time
+ std::mem::size_of::<u64>() // 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,
+11 -9
View File
@@ -70,9 +70,9 @@ impl PduParsing for ClientInfo {
let alternate_shell_size = stream.read_u16::<LittleEndian>()? as usize;
let work_dir_size = stream.read_u16::<LittleEndian>()? 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::<LittleEndian>()? 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::<LittleEndian>()? 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<Self, Self::Error> {
let bias = stream.read_u32::<LittleEndian>()?;
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::<SystemTime>::from_buffer(&mut stream)?;
let standard_bias = stream.read_u32::<LittleEndian>()?;
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::<SystemTime>::from_buffer(&mut stream)?;
let daylight_bias = stream.read_u32::<LittleEndian>()?;
@@ -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,
@@ -105,7 +105,7 @@ impl PduParsing for NewLicenseInformation {
let version = stream.read_u32::<LittleEndian>()?;
let scope_len = stream.read_u32::<LittleEndian>()?;
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::<LittleEndian>()?;
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::<LittleEndian>()?;
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,
@@ -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::<LittleEndian>()?;
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::<LittleEndian>()?;
@@ -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,
+1 -1
View File
@@ -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,
+30 -11
View File
@@ -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<String> {
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<usize> {
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;