fix(pdu): don't reject a Share Data PDU whose totalLength is under-declared

crates/ironrdp-pdu/src/rdp/headers.rs

@@ -321,8 +321,15 @@ impl<'de> Decode<'de> for ShareControlHeader {
         };
 
         if pdu_type == ShareControlPduType::DataPdu {
+            // Note this is the *re-encoded* size, not the bytes that were on the wire. The two
+            // diverge for a header-only Server Font Map: `ShareDataPdu::from_type` substitutes
+            // `FontPdu::default()` on an empty cursor, so the re-encoded size counts an 8-byte
+            // body that was never sent, and a conformant server declaring 18 was measured
+            // against 26. That is why an honest header-only Font Map was rejected, and it is
+            // the reason the exception below is keyed on the PDU type rather than on arithmetic.
             let header_length = header.size();
 
+            // An empty Update/Pointer PDU is a legitimate no-op carrying a zero length.
             let is_empty_output_pdu = matches!(
                 &header.share_control_pdu,
                 ShareControlPdu::Data(ShareDataHeader {
@@ -331,15 +338,26 @@ impl<'de> Decode<'de> for ShareControlHeader {
                 }) if data.is_empty()
             );
 
-            if header_length != total_length && !(total_length == 0 && is_empty_output_pdu) {
-                if total_length < header_length {
-                    return Err(not_enough_bytes_err!(total_length, header_length));
-                }
+            // VirtualBox's VRDP declares only the two headers of a Server Font Map (18) and
+            // never counts the 8-byte body that follows, so it under-declares by exactly the
+            // body it did send. Narrowed to this PDU type rather than allowed for Data PDUs at
+            // large, so a malformed non-output PDU declaring 1..17 is still rejected.
+            let is_font_map = matches!(
+                &header.share_control_pdu,
+                ShareControlPdu::Data(ShareDataHeader {
+                    share_data_pdu: ShareDataPdu::FontMap(_),
+                    ..
+                })
+            );
 
-                // Some Windows versions append padding that is not part of the inner unit.
+            if total_length > header_length {
+                // Over-declared: some Windows versions append padding past the inner unit.
+                // Unchanged, and still bounded by `ensure_size!`.
                 let padding = total_length - header_length;
                 ensure_size!(in: src, size: padding);
                 read_padding!(src, padding);
+            } else if total_length < header_length && !is_font_map && !is_empty_output_pdu {
+                return Err(not_enough_bytes_err!(total_length, header_length));
             }
         }
 
@@ -947,6 +965,29 @@ mod tests {
         ));
     }
 
+    /// The under-declaration carve-out must not extend past the Server Font Map case.
+    ///
+    /// A `ShutdownDenied` PDU is header-only, so 18 bytes were read; declaring 17 is neither
+    /// self-consistent with the wire nor the VRDP Font Map, and MS-RDPBCGR 3.2.5.2 asks for it
+    /// to be rejected. Guards the exact example raised in review — "a header-only PDU declared
+    /// with 1–17 bytes" — which an earlier revision of this check accepted.
+    #[test]
+    fn reject_under_declared_non_output_data_pdu() {
+        let mut encoded = zero_length_empty_data_pdu(0x25);
+        encoded[0] = 17;
+
+        let error = decode::<ShareControlHeader>(&encoded)
+            .expect_err("an under-declared length is only tolerated for a Server Font Map");
+
+        assert!(matches!(
+            error.kind(),
+            ironrdp_core::DecodeErrorKind::NotEnoughBytes {
+                received: 17,
+                expected: 18
+            }
+        ));
+    }
+
     #[test]
     fn share_data_context_retains_compression_metadata() {
         let mut user_data = encode_vec(&ShareControlHeader {

crates/ironrdp-testsuite-core/tests/pdu/rdp.rs

@@ -78,6 +78,33 @@ fn from_header_only_buffer_defaults_rdp_pdu_server_font_map() {
     assert_eq!(SERVER_FONT_MAP.clone(), decode(buf).unwrap());
 }
 
+/// VirtualBox's VRDP declares `totalLength` as the size of the two headers (18) and does not
+/// count the 8-byte Font Map body that follows it, so the PDU is complete but under-declared.
+#[test]
+fn from_buffer_with_under_declared_total_length_parses_rdp_pdu_server_font_map() {
+    let mut buf = SERVER_FONT_MAP_BUFFER;
+    buf[0] = 18;
+
+    assert_eq!(SERVER_FONT_MAP.clone(), decode(buf.as_ref()).unwrap());
+}
+
+/// A header-only Server Font Map that declares its own length honestly: 18 bytes sent, 18
+/// declared. Nothing about it is malformed, and it is the encoding a server is most likely to
+/// send in the field.
+///
+/// It is pinned separately because it is the case the old check got wrong, for a reason that is
+/// easy to reintroduce: `ShareDataPdu::from_type` defaults a `FontPdu` in when the cursor is
+/// empty, so the re-encoded size is 26 and comparing 18 against it rejected a conformant PDU.
+/// Anyone reinstating a comparison against the re-encoded size will fail here. The neighbouring
+/// `from_header_only_buffer_defaults` test does not cover it — its fixture still declares 26.
+#[test]
+fn from_header_only_buffer_with_matching_total_length_parses_rdp_pdu_server_font_map() {
+    let mut buf = SERVER_FONT_MAP_BUFFER[..18].to_vec();
+    buf[0] = 18;
+
+    assert_eq!(SERVER_FONT_MAP.clone(), decode(buf.as_slice()).unwrap());
+}
+
 #[test]
 fn from_header_only_buffer_rejects_rdp_pdu_client_font_list() {
     assert!(decode::<ironrdp_pdu::rdp::headers::ShareControlHeader>(&CLIENT_FONT_LIST_BUFFER[..18]).is_err());