Merge pull request #201 from martinling/truncated-descriptors

Fix handling of truncated descriptors

src/usb.rs

@@ -410,13 +410,13 @@ impl StandardRequest {
                 let descriptor_type =
                     DescriptorType::from((fields.value >> 8) as u8);
                 format!(
-                    "{} {} descriptor #{}{}",
+                    "{} {} #{}{}",
                     match self {
                         GetDescriptor => "Getting",
                         SetDescriptor => "Setting",
                         _ => ""
                     },
-                    descriptor_type.description(),
+                    descriptor_type.description(None),
                     fields.value & 0xFF,
                     match (descriptor_type, fields.index) {
                         (DescriptorType::String, language) if language > 0 =>
@@ -493,9 +493,9 @@ impl DescriptorType {
         }
     }
 
-    pub fn description(&self) -> &'static str {
+    fn description(&self, bytes: Option<&[u8]>) -> String {
         use DescriptorType::*;
-        match self {
+        format!("{} descriptor", match self {
             Invalid => "invalid",
             Device => "device",
             Configuration => "configuration",
@@ -510,8 +510,19 @@ impl DescriptorType {
             InterfaceAssociation => "interface association",
             BinaryObjectStore => "BOS",
             DeviceCapability => "device capability",
-            Unknown => "unknown",
-        }
+            Unknown => if let Some(type_code) = bytes.and_then(|b| b.get(1)) {
+                let type_group = match type_code {
+                    0x00..=0x1F => "standard",
+                    0x20..=0x3F => "class",
+                    0x40..=0x5F => "custom",
+                    0x60..=0xFF => "reserved",
+                };
+                return format!("{} descriptor 0x{:02X}",
+                               type_group, type_code)
+            } else {
+                "unknown"
+            }
+        })
     }
 }
 
@@ -779,7 +790,7 @@ pub enum Descriptor {
     Interface(InterfaceDescriptor),
     Endpoint(EndpointDescriptor),
     Other(DescriptorType, Vec<u8>),
-    Malformed(DescriptorType, Vec<u8>),
+    Truncated(DescriptorType, Vec<u8>),
 }
 
 impl Descriptor {
@@ -792,34 +803,18 @@ impl Descriptor {
             Endpoint(_) => "Endpoint descriptor".to_string(),
             InterfaceAssociation(_) =>
                 "Interface association descriptor".to_string(),
-            Other(desc_type, bytes) =>
-                if *desc_type == DescriptorType::Unknown {
-                    let type_code = bytes[1];
-                    let type_group = match type_code {
-                        0x00..=0x1F => "Standard",
-                        0x20..=0x3F => "Class",
-                        0x40..=0x5F => "Custom",
-                        0x60..=0xFF => "Reserved",
-                    };
-                    format!("{} descriptor 0x{:02X}, {} bytes",
-                        type_group, type_code, bytes.len())
-                } else {
-                    format!("{} descriptor, {} bytes",
-                        titlecase(desc_type.description()), bytes.len())
-                },
-            Malformed(desc_type, bytes) => {
-                let description = desc_type.description();
+            Other(desc_type, bytes) => format!("{}, {} bytes",
+                titlecase(&desc_type.description(Some(bytes))), bytes.len()),
+            Truncated(desc_type, bytes) => {
+                let description = desc_type.description(Some(bytes));
+                let desc_length = bytes[0] as usize;
                 let length = bytes.len();
-                if let Some(expected) = desc_type.expected_length() {
-                    format!(
-                        "Malformed {} descriptor (only {}/{} bytes)",
-                        description, length, expected)
-                } else {
-                    format!(
-                        "Malformed {} descriptor ({} bytes)",
-                        description, length)
-                }
-            },
+                let expected = desc_type
+                    .expected_length()
+                    .unwrap_or(desc_length);
+                format!("Truncated {} ({} of {} bytes)",
+                    description, length, expected)
+            }
         }
     }
 }
@@ -836,50 +831,73 @@ impl<'bytes> DescriptorIterator<'bytes> {
             offset: 0
         }
     }
+
+    fn decode_descriptor(
+        &mut self,
+        desc_type: DescriptorType,
+        desc_bytes: &[u8],
+    ) -> Descriptor {
+        // Decide how many bytes to decode.
+        let bytes = match desc_type.expected_length() {
+            // There aren't enough bytes for this descriptor type.
+            Some(expected) if desc_bytes.len() < expected =>
+                return Descriptor::Truncated(desc_type, desc_bytes.to_vec()),
+            // We have an expected length for this descriptor type.
+            // We'll only decode the part we're expecting.
+            Some(expected) => &desc_bytes[0 .. expected],
+            // We don't have an expected length for this descriptor type.
+            // We'll decode all the bytes as a generic descriptor.
+            None => desc_bytes,
+        };
+        match desc_type {
+            DescriptorType::Device =>
+                Descriptor::Device(
+                    DeviceDescriptor::from_bytes(bytes)),
+            DescriptorType::Configuration =>
+                Descriptor::Configuration(
+                    pod_read_unaligned::<ConfigDescriptor>(bytes)),
+            DescriptorType::Interface =>
+                Descriptor::Interface(
+                    pod_read_unaligned::<InterfaceDescriptor>(bytes)),
+            DescriptorType::Endpoint =>
+                Descriptor::Endpoint(
+                    pod_read_unaligned::<EndpointDescriptor>(bytes)),
+            DescriptorType::InterfaceAssociation =>
+                Descriptor::InterfaceAssociation(
+                    pod_read_unaligned::<InterfaceAssociationDescriptor>(bytes)),
+            _ => Descriptor::Other(desc_type, bytes.to_vec())
+        }
+    }
 }
 
 impl Iterator for DescriptorIterator<'_> {
     type Item = Descriptor;
 
     fn next(&mut self) -> Option<Descriptor> {
-        if self.offset < self.bytes.len() - 2 {
-            let remaining_bytes = &self.bytes[self.offset .. self.bytes.len()];
-            let desc_length = remaining_bytes[0] as usize;
-            let desc_type = DescriptorType::from(remaining_bytes[1]);
-            self.offset += desc_length;
-            let mut bytes = &remaining_bytes[0 .. desc_length];
-            if let Some(expected) = desc_type.expected_length() {
-                // If there aren't enough bytes for the descriptor type, it is
-                // malformed and we can't interpret it.
-                if desc_length < expected {
-                    return Some(Descriptor::Malformed(desc_type, bytes.to_vec()))
+        use Descriptor::Truncated;
+        use DescriptorType::Unknown;
+        let remaining = self.bytes.len() - self.offset;
+        let (descriptor, bytes_consumed) = match remaining {
+            // All bytes consumed by descriptors, none left over.
+            0 => return None,
+            // Not enough bytes for type and length.
+            1 => (Truncated(Unknown, self.bytes[self.offset..].to_vec()), 1),
+            _ => {
+                let remaining_bytes = &self.bytes[self.offset..];
+                let desc_length = remaining_bytes[0] as usize;
+                let desc_type = DescriptorType::from(remaining_bytes[1]);
+                if desc_length > remaining {
+                    // We don't have all the bytes of this descriptor.
+                    (Truncated(desc_type, remaining_bytes.to_vec()), remaining)
+                } else {
+                    // This looks like a valid descriptor, decode it.
+                    let bytes = &remaining_bytes[0 .. desc_length];
+                    (self.decode_descriptor(desc_type, bytes), desc_length)
                 }
-                // The expected length is the minimum length, but sometimes the
-                // descriptors have extra padding/garbage data at the end.
-                // Handle this by trimming off the extra data.
-                bytes = &bytes[0 .. expected];
-            };
-            return Some(match desc_type {
-                DescriptorType::Device =>
-                    Descriptor::Device(
-                        DeviceDescriptor::from_bytes(bytes)),
-                DescriptorType::Configuration =>
-                    Descriptor::Configuration(
-                        pod_read_unaligned::<ConfigDescriptor>(bytes)),
-                DescriptorType::Interface =>
-                    Descriptor::Interface(
-                        pod_read_unaligned::<InterfaceDescriptor>(bytes)),
-                DescriptorType::Endpoint =>
-                    Descriptor::Endpoint(
-                        pod_read_unaligned::<EndpointDescriptor>(bytes)),
-                DescriptorType::InterfaceAssociation =>
-                    Descriptor::InterfaceAssociation(
-                        pod_read_unaligned::<InterfaceAssociationDescriptor>(bytes)),
-                _ => Descriptor::Other(desc_type, bytes.to_vec())
-            });
-        }
-        // Not enough data for another descriptor.
-        None
+            }
+        };
+        self.offset += bytes_consumed;
+        Some(descriptor)
     }
 }
 

tests/bad-descriptor-length/devices-reference.txt

@@ -0,0 +1,124 @@
+Device 16: Unknown
+ No device descriptor
+ Configuration 1
+  Configuration descriptor
+   Length: 9 bytes
+   Type: 0x02
+   Total length: 285 bytes
+   Number of interfaces: 3
+   Configuration number: 1
+   Configuration string: #4 (not seen)
+   Attributes: 0xA0
+   Max power: 100mA
+  Function 5: Audio
+   Interface association descriptor
+    Length: 8 bytes
+    Type: 0x0B
+    First interface: 0
+    Interface count: 2
+    Function class: 0x01: Audio
+    Function subclass: 0x00
+    Function protocol: 0x20
+    Function number: 5
+   Interface 0: Audio
+    Interface descriptor
+     Length: 9 bytes
+     Type: 0x04
+     Interface number: 0
+     Alternate setting: 0
+     Number of endpoints: 1
+     Class: 0x01: Audio
+     Subclass: 0x01: Control Device
+     Protocol: 0x20
+     Interface string: #5 (not seen)
+    Class descriptor 0x24, 9 bytes
+    Class descriptor 0x24, 8 bytes
+    Class descriptor 0x24, 17 bytes
+    Class descriptor 0x24, 12 bytes
+    Class descriptor 0x24, 18 bytes
+    Class descriptor 0x24, 16 bytes
+    Endpoint 7 IN (interrupt)
+     Endpoint descriptor
+      Length: 7 bytes
+      Type: 0x05
+      Endpoint address: 0x87
+      Attributes: 0x03
+      Max packet size: 16 bytes
+      Interval: 0x08
+   Interface 1: Audio
+    Interface descriptor
+     Length: 9 bytes
+     Type: 0x04
+     Interface number: 1
+     Alternate setting: 0
+     Number of endpoints: 0
+     Class: 0x01: Audio
+     Subclass: 0x02: Streaming
+     Protocol: 0x20
+     Interface string: #5 (not seen)
+   Interface 1 alt 1: Audio
+    Interface descriptor
+     Length: 9 bytes
+     Type: 0x04
+     Interface number: 1
+     Alternate setting: 1
+     Number of endpoints: 1
+     Class: 0x01: Audio
+     Subclass: 0x02: Streaming
+     Protocol: 0x20
+     Interface string: #5 (not seen)
+    Class descriptor 0x24, 16 bytes
+    Class descriptor 0x24, 6 bytes
+    Endpoint 4 OUT (isochronous)
+     Endpoint descriptor
+      Length: 7 bytes
+      Type: 0x05
+      Endpoint address: 0x04
+      Attributes: 0x09
+      Max packet size: 248 bytes
+      Interval: 0x01
+     Class descriptor 0x25, 8 bytes
+   Interface 1 alt 2: Audio
+    Interface descriptor
+     Length: 9 bytes
+     Type: 0x04
+     Interface number: 1
+     Alternate setting: 2
+     Number of endpoints: 1
+     Class: 0x01: Audio
+     Subclass: 0x02: Streaming
+     Protocol: 0x20
+     Interface string: #5 (not seen)
+    Class descriptor 0x24, 16 bytes
+    Class descriptor 0x24, 6 bytes
+    Endpoint 4 OUT (isochronous)
+     Endpoint descriptor
+      Length: 7 bytes
+      Type: 0x05
+      Endpoint address: 0x04
+      Attributes: 0x09
+      Max packet size: 372 bytes
+      Interval: 0x01
+     Class descriptor 0x25, 8 bytes
+   Interface 1 alt 3: Audio
+    Interface descriptor
+     Length: 9 bytes
+     Type: 0x04
+     Interface number: 1
+     Alternate setting: 3
+     Number of endpoints: 1
+     Class: 0x01: Audio
+     Subclass: 0x02: Streaming
+     Protocol: 0x20
+     Interface string: #5 (not seen)
+    Class descriptor 0x24, 16 bytes
+    Class descriptor 0x24, 6 bytes
+    Endpoint 4 OUT (isochronous)
+     Endpoint descriptor
+      Length: 7 bytes
+      Type: 0x05
+      Endpoint address: 0x04
+      Attributes: 0x09
+      Max packet size: 496 bytes
+      Interval: 0x01
+     Truncated class descriptor 0x25 (3 of 8 bytes)

tests/bad-descriptor-length/reference.txt

@@ -0,0 +1,37 @@
+Getting configuration descriptor #0 for device 16, reading 255 bytes
+ SETUP transaction on 16.0 with 8 data bytes, ACK: [80, 06, 00, 02, 00, 00, FF, 00]
+  SETUP packet on 16.0, CRC 1E
+  DATA0 packet with CRC A4E9 and 8 data bytes: [80, 06, 00, 02, 00, 00, FF, 00]
+  ACK packet
+ 9 times: IN transaction on 16.0, NAK
+  IN packet on 16.0, CRC 1E
+  NAK packet
+ IN transaction on 16.0 with 64 data bytes, ACK: [09, 02, 1D, 01, 03, 01, 04, A0, 32, 08, 0B, 00, 02, 01, 00, 20, 05, 09, 04, 00, 00, 01, 01, 01, 20, 05, 09, 24, 01, 00, 02, 04, 50, 00, 00, 08, 24, 0A, 04, 07, 07, 00, 00, 11, 24, 02, 0A, 01, 01, 00, 04, 02, 03, 00, 00, 00, 00, 00, 00, 00, 0C, 24, 03, 10]
+  IN packet on 16.0, CRC 1E
+  DATA1 packet with CRC C464 and 64 data bytes: [09, 02, 1D, 01, 03, 01, 04, A0, 32, 08, 0B, 00, 02, 01, 00, 20, 05, 09, 04, 00, 00, 01, 01, 01, 20, 05, 09, 24, 01, 00, 02, 04, 50, 00, 00, 08, 24, 0A, 04, 07, 07, 00, 00, 11, 24, 02, 0A, 01, 01, 00, 04, 02, 03, 00, 00, 00, 00, 00, 00, 00, 0C, 24, 03, 10]
+  ACK packet
+ 5 times: IN transaction on 16.0, NAK
+  IN packet on 16.0, CRC 1E
+  NAK packet
+ IN transaction on 16.0 with 64 data bytes, ACK: [02, 03, 0A, 16, 04, 00, 00, 00, 12, 24, 06, 16, 0A, 03, 00, 00, 00, 0C, 00, 00, 00, 0C, 00, 00, 00, 00, 10, 24, 09, 19, DA, 0B, 01, 16, 02, 03, 00, 00, 00, 00, 00, 00, 07, 05, 87, 03, 10, 00, 08, 09, 04, 01, 00, 00, 01, 02, 20, 05, 09, 04, 01, 01, 01, 01]
+  IN packet on 16.0, CRC 1E
+  DATA0 packet with CRC 0D37 and 64 data bytes: [02, 03, 0A, 16, 04, 00, 00, 00, 12, 24, 06, 16, 0A, 03, 00, 00, 00, 0C, 00, 00, 00, 0C, 00, 00, 00, 00, 10, 24, 09, 19, DA, 0B, 01, 16, 02, 03, 00, 00, 00, 00, 00, 00, 07, 05, 87, 03, 10, 00, 08, 09, 04, 01, 00, 00, 01, 02, 20, 05, 09, 04, 01, 01, 01, 01]
+  ACK packet
+ 10 times: IN transaction on 16.0, NAK
+  IN packet on 16.0, CRC 1E
+  NAK packet
+ IN transaction on 16.0 with 64 data bytes, ACK: [02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00, 00, 06, 24, 02, 01, 02, 10, 07, 05, 04, 09, F8, 00, 01, 08, 25, 01, 00, 00, 00, 00, 00, 09, 04, 01, 02, 01, 01, 02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00]
+  IN packet on 16.0, CRC 1E
+  DATA1 packet with CRC BBBE and 64 data bytes: [02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00, 00, 06, 24, 02, 01, 02, 10, 07, 05, 04, 09, F8, 00, 01, 08, 25, 01, 00, 00, 00, 00, 00, 09, 04, 01, 02, 01, 01, 02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00]
+  ACK packet
+ 16 times: IN transaction on 16.0, NAK
+  IN packet on 16.0, CRC 1E
+  NAK packet
+ IN transaction on 16.0 with 63 data bytes, ACK: [00, 06, 24, 02, 01, 03, 18, 07, 05, 04, 09, 74, 01, 01, 08, 25, 01, 00, 00, 00, 00, 00, 09, 04, 01, 03, 01, 01, 02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00, 00, 06, 24, 02, 01, 04, 20, 07, 05, 04, 09, F0, 01, 01, 08, 25, 01]
+  IN packet on 16.0, CRC 1E
+  DATA0 packet with CRC 1E7A and 63 data bytes: [00, 06, 24, 02, 01, 03, 18, 07, 05, 04, 09, 74, 01, 01, 08, 25, 01, 00, 00, 00, 00, 00, 09, 04, 01, 03, 01, 01, 02, 20, 05, 10, 24, 01, 0A, 00, 01, 01, 00, 00, 00, 02, 03, 00, 00, 00, 00, 06, 24, 02, 01, 04, 20, 07, 05, 04, 09, F0, 01, 01, 08, 25, 01]
+  ACK packet
+ OUT transaction on 16.0 with no data, ACK
+  OUT packet on 16.0, CRC 1E
+  DATA1 packet with CRC 0000 and no data
+  ACK packet

tests/tests.txt

@@ -1,5 +1,6 @@
 analyzer-test-bad-cable
 bad-crcs
+bad-descriptor-length
 emf2022-badge
 hackrf-connect
 hackrf-dfu-enum