From 9c3ebfb410cd8307eee4314ed583250eb7a46cb2 Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 10 Sep 2021 14:17:15 -0700 Subject: [PATCH] usb_util: validate bLength in next_descriptor When skipping descriptors in the next_descriptor() helper function, we advance the offset in the input bytestream by adding the user-controlled bLength field. If bLength was 0, next_descriptor() would get stuck in a loop and never return. Add a check for this case as well as a unit test based on the failing fuzzer input. BUG=b:198320695 TEST=cargo test -p usb_util TEST=cros_fuzz Change-Id: Iec130a33b28f05219907265b7acafa9ee3791c1a Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3155363 Reviewed-by: Dennis Kempin Tested-by: kokoro Commit-Queue: Daniel Verkamp --- usb_util/src/descriptor.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/usb_util/src/descriptor.rs b/usb_util/src/descriptor.rs index 4d9a3fb993..1ec8fa25e8 100644 --- a/usb_util/src/descriptor.rs +++ b/usb_util/src/descriptor.rs @@ -153,6 +153,12 @@ pub fn parse_usbfs_descriptors(data: &[u8]) -> Result { { return Err(Error::DescriptorParse); } + + // Make sure we make forward progress. + if hdr.bLength == 0 { + return Err(Error::DescriptorParse); + } + // Skip this entire descriptor, since it's not the right type. *offset += hdr.bLength as usize; } @@ -652,4 +658,19 @@ mod tests { assert_eq!(i.bInterfaceProtocol, 0x00); assert_eq!(i.iInterface, 0x00); } + + #[test] + fn parse_descriptors_length_0() { + // Device descriptor followed by a bogus descriptor with bLength == 0. + // Note that this was generated by a fuzzer, so field values may not make sense. + let data: &[u8] = &[ + 0x10, 0x00, 0x18, 0x25, 0x80, 0x80, 0xAC, 0x03, 0x22, 0x05, 0x00, 0x00, 0x00, 0x00, + 0xC3, 0x2A, 0x00, 0x32, 0x00, + ]; + + let d = parse_usbfs_descriptors(data); + if !d.is_err() { + panic!("parse_usbfs_descriptors should have failed"); + } + } }