Fix memory safety bugs in PeiUsbGetAllConfiguration

Fixes oss-fuzz issue #70813. ==9957==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7e248c3009d8 at pc 0x00000059cb32 bp 0x7fff07ef3590 sp 0x7fff07ef3588 WRITE of size 8 at 0x7e248c3009d8 thread T0 SCARINESS: 57 (8-byte-write-stack-buffer-overflow) #0 0x59cb31 in PeiUsbGetAllConfiguration edk2/MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c:928:61 intel#1 0x596c4b in RunTestHarness hbfa-fl/HBFA/UefiHostFuzzTestCasePkg/TestCase/MdeModulePkg/Bus/Usb/UsbBusPei/TestPeiUsb.c:74:3 Signed-off-by: Tamas K Lengyel <[email protected]>
tklengyel · Sep 11, 2024 · 355d8d5 · 355d8d5
1 parent 0224f27
commit 355d8d5
Showing 1 changed file with 45 additions and 0 deletions.
diff --git a/edk2-bugfixes/0001-UsbBusPei-Error-out-when-configuration-exceeds-maxim.patch b/edk2-bugfixes/0001-UsbBusPei-Error-out-when-configuration-exceeds-maxim.patch
@@ -0,0 +1,45 @@
+From 8be3e7888ab800a1ee236c616502f042c66f86d0 Mon Sep 17 00:00:00 2001
+From: Tamas K Lengyel <[email protected]>
+Date: Tue, 10 Sep 2024 18:22:20 +0000
+Subject: [PATCH] UsbBusPei: Error out when configuration exceeds maximums
+ allowed
+
+ASSERT is not appropriate for a security check, switch to DEBUG and
+return error code.
+
+Signed-off-by: Tamas K Lengyel <[email protected]>
+---
+ MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c | 10 +++++++++-
+ 1 file changed, 9 insertions(+), 1 deletion(-)
+
+diff --git a/MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c b/MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c
+index 6ea4495162..46201eaa9f 100644
+--- a/MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c
++++ b/MdeModulePkg/Bus/Usb/UsbBusPei/UsbPeim.c
+@@ -871,6 +871,11 @@ PeiUsbGetAllConfiguration (
+   Ptr       += sizeof (EFI_USB_CONFIG_DESCRIPTOR);
+   LengthLeft = ConfigDescLength - SkipBytes - sizeof (EFI_USB_CONFIG_DESCRIPTOR);
+
++  if (PeiUsbDevice->ConfigDesc->NumInterfaces > MAX_INTERFACE) {
++    DEBUG ((DEBUG_ERROR, "PeiUsbGet Number of interfaces in the configuration exceeds maximum allowed"));
++    return EFI_DEVICE_ERROR;
++  }
++
+   for (InterfaceIndex = 0; InterfaceIndex < PeiUsbDevice->ConfigDesc->NumInterfaces; InterfaceIndex++) {
+     //
+     // Get the interface descriptor
+@@ -902,7 +907,10 @@ PeiUsbGetAllConfiguration (
+     // Parse all the endpoint descriptor within this interface
+     //
+     NumOfEndpoint = PeiUsbDevice->InterfaceDescList[InterfaceIndex]->NumEndpoints;
+-    ASSERT (NumOfEndpoint <= MAX_ENDPOINT);
++    if (NumOfEndpoint > MAX_ENDPOINT) {
++        DEBUG ((DEBUG_ERROR, "PeiUsbGet Number of endpoints in interface configuration exceeds maximum allowed"));
++        return EFI_DEVICE_ERROR;
++    }
+
+     for (Index = 0; Index < NumOfEndpoint; Index++) {
+       //
+-- 
+2.34.1
+