MdeModulePkg/UsbBus: Fix out-of-bound read access to descriptors

Today's implementation reads the Type/Length field in the USB
descriptors data without checking whether the offset to read is
beyond the data boundary.

The patch fixes this issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>

diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
index 061e4622e82b25c9a21c467ded70c3eeb30031f5..70442c57dabf211adc0b0cda64d86c849fd20f2c 100644 (file)
--- a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
+++ b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c
@@ -177,6 +177,18 @@ UsbCreateDesc (
     DescLen = sizeof (EFI_USB_ENDPOINT_DESCRIPTOR);\r
     CtrlLen = sizeof (USB_ENDPOINT_DESC);\r
     break;\r
+\r
+  default:\r
+    ASSERT (FALSE);\r
+    return NULL;\r
+  }\r
+\r
+  //\r
+  // Total length is too small that cannot hold the single descriptor header plus data. \r
+  //\r
+  if (Len <= sizeof (USB_DESC_HEAD)) {\r
+    DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, total length = %d!\n", Len));\r
+    return NULL;\r
   }\r
 \r
   //\r
@@ -184,24 +196,43 @@ UsbCreateDesc (
   // format. Skip the descriptor that isn't of this Type\r
   //\r
   Offset = 0;\r
-  Head   = (USB_DESC_HEAD*)DescBuf;\r
+  Head   = (USB_DESC_HEAD *)DescBuf;\r
+  while (Offset < Len - sizeof (USB_DESC_HEAD)) {\r
+    //\r
+    // Above condition make sure Head->Len and Head->Type are safe to access\r
+    //\r
+    Head = (USB_DESC_HEAD *)&DescBuf[Offset];\r
 \r
-  while ((Offset < Len) && (Head->Type != Type)) {\r
-    Offset += Head->Len;\r
-    if (Len <= Offset) {\r
-      DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Beyond boundary!\n"));\r
+    if (Head->Len == 0) {\r
+      DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n"));\r
       return NULL;\r
     }\r
-    Head    = (USB_DESC_HEAD*)(DescBuf + Offset);\r
-    if (Head->Len == 0) {\r
-      DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n"));\r
+\r
+    //\r
+    // Make sure no overflow when adding Head->Len to Offset.\r
+    //\r
+    if (Head->Len > MAX_UINTN - Offset) {\r
+      DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = %d!\n", Head->Len));\r
       return NULL;\r
     }\r
+\r
+    Offset += Head->Len;\r
+\r
+    if (Head->Type == Type) {\r
+      break;\r
+    }\r
+  }\r
+\r
+  //\r
+  // Head->Len is invalid resulting data beyond boundary, or\r
+  // Descriptor cannot be found: No such type.\r
+  //\r
+  if (Len < Offset) {\r
+    DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Offset/Len = %d/%d!\n", Offset, Len));\r
   }\r
 \r
-  if ((Len <= Offset)      || (Len < Offset + Head->Len) ||\r
-      (Head->Type != Type) || (Head->Len < DescLen)) {\r
-    DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));\r
+  if ((Head->Type != Type) || (Head->Len < DescLen)) {\r
+    DEBUG ((DEBUG_ERROR, "UsbCreateDesc: descriptor cannot be found, Header(T/L) = %d/%d!\n", Head->Type, Head->Len));\r
     return NULL;\r
   }\r
 \r
@@ -212,7 +243,7 @@ UsbCreateDesc (
 \r
   CopyMem (Desc, Head, (UINTN) DescLen);\r
 \r
-  *Consumed = Offset + Head->Len;\r
+  *Consumed = Offset;\r
 \r
   return Desc;\r
 }\r