[v11] Fix directory sharing for non-ascii directory names

lib/srv/desktop/rdp/rdpclient/src/rdpdr.rs

@@ -1965,39 +1965,18 @@ impl ClientDeviceListAnnounceRequest {
     // We only need to announce the smartcard in this Client Device List Announce Request.
     // Drives (directories) can be announced at any time with a Client Drive Device List Announce.
     fn new_smartcard(device_id: u32) -> Self {
-        Self {
-            device_count: 1,
-            device_list: vec![DeviceAnnounceHeader {
-                device_type: DeviceType::RDPDR_DTYP_SMARTCARD,
-                device_id,
-                // This name is a constant defined by the spec.
-                preferred_dos_name: "SCARD".to_string(),
-                device_data_length: 0,
-                device_data: vec![],
-            }],
-        }
+        Self::new(vec![DeviceAnnounceHeader::new_smartcard(device_id)])
     }
 
     /// A new drive can be announced at any time during RDP's operation. It is up to the caller
-    pub fn new_drive(device_id: u32, drive_name: String) -> Self {
-        // If the client supports DRIVE_CAPABILITY_VERSION_02 in the Drive Capability Set,
-        // then the full name MUST also be specified in the DeviceData field, as a null-terminated
-        // Unicode string. If the DeviceDataLength field is nonzero, the content of the
-        // PreferredDosName field is ignored.
-        //
-        // In the RDP spec, Unicode typically means null-terminated UTF-16LE, however empirically it
-        // appears that this field expects null-terminated UTF-8.
-        let device_data = util::to_utf8(&drive_name);
+    pub fn new_drive(device_id: u32, name: String) -> Self {
+        Self::new(vec![DeviceAnnounceHeader::new_drive(device_id, name)])
+    }
 
+    fn new(device_list: Vec<DeviceAnnounceHeader>) -> Self {
         Self {
-            device_count: 1,
-            device_list: vec![DeviceAnnounceHeader {
-                device_type: DeviceType::RDPDR_DTYP_FILESYSTEM,
-                device_id,
-                preferred_dos_name: drive_name,
-                device_data_length: device_data.len() as u32,
-                device_data,
-            }],
+            device_count: device_list.len() as u32,
+            device_list,
         }
     }
 }
@@ -2017,27 +1996,72 @@ impl Encode for ClientDeviceListAnnounceRequest {
 /// https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/32e34332-774b-4ead-8c9d-5d64720d6bf9
 #[derive(Debug)]
 struct DeviceAnnounceHeader {
+    name: String,
     device_type: DeviceType,
     device_id: u32,
-    preferred_dos_name: String,
-    device_data_length: u32,
-    device_data: Vec<u8>,
 }
 
 impl DeviceAnnounceHeader {
+    fn new_smartcard(device_id: u32) -> Self {
+        Self {
+            name: "".to_string(), // ignored for smartcards
+            device_type: DeviceType::RDPDR_DTYP_SMARTCARD,
+            device_id,
+        }
+    }
+
+    fn new_drive(device_id: u32, name: String) -> Self {
+        Self {
+            name,
+            device_type: DeviceType::RDPDR_DTYP_FILESYSTEM,
+            device_id,
+        }
+    }
+
     fn encode(&self) -> RdpResult<Message> {
         let mut w = vec![];
         w.write_u32::<LittleEndian>(self.device_type.to_u32().unwrap())?;
         w.write_u32::<LittleEndian>(self.device_id)?;
-        let mut name: &str = &self.preferred_dos_name;
-        // See "PreferredDosName" at
-        // https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/32e34332-774b-4ead-8c9d-5d64720d6bf9
-        if name.len() > 7 {
-            name = &name[..7];
+        // Initially preferred_dos_name was set to the first 7 ASCII characters (per the spec)
+        // of the drive name when the drive was of type DeviceType::RDPDR_DTYP_FILESYSTEM,
+        // however this was causing a panic when the user shared a directory with a name with
+        // non-ASCII characters (e.g. 中文测试).
+        let mut preferred_dos_name: &str = match self.device_type {
+            // In this case we can just use any random value, since it will be ignored in favor
+            // of the UTF-8 encoded drive name in the DeviceData field.
+            //
+            // We may need to revisit this if we ever support multiple drives
+            // (i.e. do "FILE1", "FILE2", etc).
+            DeviceType::RDPDR_DTYP_FILESYSTEM => "FILE",
+            // If DeviceType is set to RDPDR_DTYP_SMARTCARD, the PreferredDosName MUST be set to "SCARD".
+            // See: https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-rdpefs/32e34332-774b-4ead-8c9d-5d64720d6bf9
+            DeviceType::RDPDR_DTYP_SMARTCARD => "SCARD",
+            _ => {
+                return Err(RdpError::TryError(format!(
+                    "unsupported device type {device_type:?}",
+                    device_type = self.device_type
+                )))
+            }
+        };
+        // Sanity check to make sure we don't write more than
+        // 8 bytes (including the null terminator) for PreferredDosName.
+        if preferred_dos_name.len() > 7 {
+            preferred_dos_name = &preferred_dos_name[..7];
         }
-        w.extend_from_slice(&format!("{name:\x00<8}").into_bytes());
-        w.write_u32::<LittleEndian>(self.device_data_length)?;
-        w.extend_from_slice(&self.device_data);
+        w.extend_from_slice(&format!("{preferred_dos_name:\x00<8}").into_bytes());
+
+        // If the client supports DRIVE_CAPABILITY_VERSION_02 in the Drive Capability Set (which we do),
+        // then the full name MUST also be specified in the DeviceData field, as a null-terminated
+        // Unicode string. If the DeviceDataLength field is nonzero, the content of the
+        // PreferredDosName field is ignored.
+        //
+        // In the RDP spec, Unicode typically means null-terminated UTF-16LE, however empirically it
+        // appears that this field expects null-terminated UTF-8.
+        let device_data = util::to_utf8(&self.name);
+        let device_data_length = device_data.len() as u32;
+
+        w.write_u32::<LittleEndian>(device_data_length)?;
+        w.extend_from_slice(&device_data);
         Ok(w)
     }
 }

lib/srv/desktop/rdp/rdpclient/src/rdpdr/tests.rs

@@ -306,16 +306,9 @@ fn test_handle_client_id_confirm() {
         },
         vec![(
             PacketId::PAKID_CORE_DEVICELIST_ANNOUNCE,
-            Box::new(ClientDeviceListAnnounceRequest {
-                device_count: 1,
-                device_list: vec![DeviceAnnounceHeader {
-                    device_type: DeviceType::RDPDR_DTYP_SMARTCARD,
-                    device_id: 1,
-                    preferred_dos_name: "SCARD".to_string(),
-                    device_data_length: 0,
-                    device_data: vec![],
-                }],
-            }),
+            Box::new(ClientDeviceListAnnounceRequest::new_smartcard(
+                SCARD_DEVICE_ID,
+            )),
         )],
     );
 
@@ -429,3 +422,18 @@ fn check_dir_sharing_methods_error_when_disabled() {
         }
     }
 }
+
+/// Checks that we can encode a DeviceAnnounceHeader with a non-ascii name,
+/// which was causing a panic in the past.
+#[test]
+fn test_device_announce_header_encode_with_non_ascii() {
+    assert_eq!(
+        DeviceAnnounceHeader::new_drive(2, "中文测试".to_string())
+            .encode()
+            .unwrap(),
+        vec![
+            8, 0, 0, 0, 2, 0, 0, 0, 70, 73, 76, 69, 0, 0, 0, 0, 13, 0, 0, 0, 228, 184, 173, 230,
+            150, 135, 230, 181, 139, 232, 175, 149, 0
+        ]
+    )
+}