diff options
| author | Ruiyu Ni <ruiyu.ni@intel.com> | 2018-09-27 16:34:36 +0800 |
|---|---|---|
| committer | Ruiyu Ni <ruiyu.ni@intel.com> | 2018-10-17 11:03:56 +0800 |
| commit | 4c034bf62cbc1f3c5f4b5df25de97f0f528132b2 (patch) | |
| tree | ff253b3bfc33a6c4b340c38f3380853694e19a28 | |
| parent | 4f8b2f9d727d0035a2ca8d7371629418684bf80f (diff) | |
| download | edk2-4c034bf62cbc1f3c5f4b5df25de97f0f528132b2.tar.gz edk2-4c034bf62cbc1f3c5f4b5df25de97f0f528132b2.tar.bz2 edk2-4c034bf62cbc1f3c5f4b5df25de97f0f528132b2.zip | |
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>
| -rw-r--r-- | MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c | 55 |
1 files changed, 43 insertions, 12 deletions
diff --git a/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c b/MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.c index 061e4622e8..70442c57da 100644 --- 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);
CtrlLen = sizeof (USB_ENDPOINT_DESC);
break;
+
+ default:
+ ASSERT (FALSE);
+ return NULL;
+ }
+
+ //
+ // Total length is too small that cannot hold the single descriptor header plus data.
+ //
+ if (Len <= sizeof (USB_DESC_HEAD)) {
+ DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, total length = %d!\n", Len));
+ return NULL;
}
//
@@ -184,24 +196,43 @@ UsbCreateDesc ( // format. Skip the descriptor that isn't of this Type
//
Offset = 0;
- Head = (USB_DESC_HEAD*)DescBuf;
+ Head = (USB_DESC_HEAD *)DescBuf;
+ while (Offset < Len - sizeof (USB_DESC_HEAD)) {
+ //
+ // Above condition make sure Head->Len and Head->Type are safe to access
+ //
+ Head = (USB_DESC_HEAD *)&DescBuf[Offset];
- while ((Offset < Len) && (Head->Type != Type)) {
- Offset += Head->Len;
- if (Len <= Offset) {
- DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Beyond boundary!\n"));
+ if (Head->Len == 0) {
+ DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n"));
return NULL;
}
- Head = (USB_DESC_HEAD*)(DescBuf + Offset);
- if (Head->Len == 0) {
- DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = 0!\n"));
+
+ //
+ // Make sure no overflow when adding Head->Len to Offset.
+ //
+ if (Head->Len > MAX_UINTN - Offset) {
+ DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Head->Len = %d!\n", Head->Len));
return NULL;
}
+
+ Offset += Head->Len;
+
+ if (Head->Type == Type) {
+ break;
+ }
+ }
+
+ //
+ // Head->Len is invalid resulting data beyond boundary, or
+ // Descriptor cannot be found: No such type.
+ //
+ if (Len < Offset) {
+ DEBUG ((DEBUG_ERROR, "UsbCreateDesc: met mal-format descriptor, Offset/Len = %d/%d!\n", Offset, Len));
}
- if ((Len <= Offset) || (Len < Offset + Head->Len) ||
- (Head->Type != Type) || (Head->Len < DescLen)) {
- DEBUG (( EFI_D_ERROR, "UsbCreateDesc: met mal-format descriptor\n"));
+ if ((Head->Type != Type) || (Head->Len < DescLen)) {
+ DEBUG ((DEBUG_ERROR, "UsbCreateDesc: descriptor cannot be found, Header(T/L) = %d/%d!\n", Head->Type, Head->Len));
return NULL;
}
@@ -212,7 +243,7 @@ UsbCreateDesc ( CopyMem (Desc, Head, (UINTN) DescLen);
- *Consumed = Offset + Head->Len;
+ *Consumed = Offset;
return Desc;
}
|