uboot USB stack security issue during parsing of usb hub descriptor

[Tom Rini]

On Mon, Nov 02, 2020 at 02:53:24PM -0300, Enrique Nissim wrote:

> Hi everyone,
> 
> My name is Enrique Nissim... After exchanging some emails with Marek and
> Tom, I'm sharing here what I believe could be a security issue in the USB
> stack of uboot. Specifically, in the function that parses the USB HUB
> Descriptors during enumeration:
> 
> https://github.com/u-boot/u-boot/blob/c2279d784e35fa25ee3a9fa28a74a1ba545f8c1e/common/usb_hub.c#L613
> 
> File: common\usb_hub.c
> 613: static int usb_hub_configure(struct usb_device *dev)
> 614: {
> 615: int i, length;
> 616: ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);
> 617: unsigned char *bitmap;
> 618: short hubCharacteristics;
> 619: struct usb_hub_descriptor *descriptor;
> 620: struct usb_hub_device *hub;
> 621: struct usb_hub_status *hubsts;
> 622: int ret;
> 623:
> 624: hub = usb_get_hub_device(dev);
> 625: if (hub == NULL)
> 626: return -ENOMEM;
> 627: hub->pusb_dev = dev;
> 628:
> 629: /* Get the the hub descriptor */
> 630: ret = usb_get_hub_descriptor(dev, buffer, 4);
> 631: if (ret < 0) {
> 632: debug("usb_hub_configure: failed to get hub " \
> 633:      "descriptor, giving up %lX\n", dev->status);
> 634: return ret;
> 635: }
> 636: descriptor = (struct usb_hub_descriptor *)buffer;
> 637:
> 638: length = min_t(int, descriptor->bLength,
> 639:       sizeof(struct usb_hub_descriptor));
> 640:
> 641: ret = usb_get_hub_descriptor(dev, buffer, length);
> 642: if (ret < 0) {
> 643: debug("usb_hub_configure: failed to get hub " \
> 644:      "descriptor 2nd giving up %lX\n", dev->status);
> 645: return ret;
> 646: }
> 647: memcpy((unsigned char *)&hub->desc, buffer, length);
> 648: /* adjust 16bit values */
> 649: put_unaligned(le16_to_cpu(get_unaligned(
> 650: &descriptor->wHubCharacteristics)),
> 651: &hub->desc.wHubCharacteristics);
> 652: /* set the bitmap */
> 653: bitmap = (unsigned char *)&hub->desc.u.hs.DeviceRemovable[0];
> 654: /* devices not removable by default */
> 655: memset(bitmap, 0xff, (USB_MAXCHILDREN+1+7)/8);
> 656: bitmap = (unsigned char *)&hub->desc.u.hs.PortPowerCtrlMask[0];
> 657: memset(bitmap, 0xff, (USB_MAXCHILDREN+1+7)/8); /* PowerMask = 1B */
> 658:
> 659: for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++)
> 660: hub->desc.u.hs.DeviceRemovable[i] =
> 661: descriptor->u.hs.DeviceRemovable[i];
> 662:
> 663: for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++)
> 664: hub->desc.u.hs.PortPowerCtrlMask[i] =
> 665: descriptor->u.hs.PortPowerCtrlMask[i];
> 666:
> 667: dev->maxchild = descriptor->bNbrPorts;
> 668: debug("%d ports detected\n", dev->maxchild);
> 669:
> 670: hubCharacteristics = get_unaligned(&hub->desc.wHubCharacteristics);
> 
> 
> The function starts by allocating a buffer on the stack of 512 bytes in
> size. This memory isn't initialized in any way, which means it contains
> `random` garbage values from the stack.
> 
> There are two problems in this function:
> 
> 1) The first time the hub descriptor is retrieved (line 630), a device
> could provide a bLength value of 1, in which case the length variable will
> be assigned 1 and after retrieving the descriptor again and copying the
> buffer content into hub->desc, uses of further members like
> `descriptor->u.hs.DeviceRemovable[i]`,
> `descriptor->u.hs.PortPowerCtrlMask[i]`, or `descriptor->bNbrPorts` will
> all be out of bounds in terms and contain garbage values from the stack.
> 
> 2) By providing a full hub descriptor and setting bNbrPorts to 0xf0, the
> for loops on lines 659 and 663 will be copying data out of bounds the
> hub->desc structure, thus corrupting memory.
> 
> I've checked the master branch of the usb core driver for Linux and I saw
> there are more checks around this same value:
> https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L1411

Good catch.  Can you send a patch?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot-custodians/attachments/20201103/f213be4c/attachment.sig>