<div dir="ltr">Hi everyone, <br><br>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:<br><br><a href="https://github.com/u-boot/u-boot/blob/c2279d784e35fa25ee3a9fa28a74a1ba545f8c1e/common/usb_hub.c#L613">https://github.com/u-boot/u-boot/blob/c2279d784e35fa25ee3a9fa28a74a1ba545f8c1e/common/usb_hub.c#L613</a><div><br>File: common\usb_hub.c<br>613: static int usb_hub_configure(struct usb_device *dev)<br>614: {<br>615:       int i, length;<br>616:    ALLOC_CACHE_ALIGN_BUFFER(unsigned char, buffer, USB_BUFSIZ);<br>617:      unsigned char *bitmap;<br>618:    short hubCharacteristics;<br>619:         struct usb_hub_descriptor *descriptor;<br>620:    struct usb_hub_device *hub;<br>621:       struct usb_hub_status *hubsts;<br>622:    int ret;<br>623: <br>624:   hub = usb_get_hub_device(dev);<br>625:    if (hub == NULL)<br>626:          return -ENOMEM;<br>627:   hub->pusb_dev = dev;<br>628: <br>629:    /* Get the the hub descriptor */<br>630:  ret = usb_get_hub_descriptor(dev, buffer, 4);<br>631:     if (ret < 0) {<br>632:                 debug("usb_hub_configure: failed to get hub " \<br>633:                       "descriptor, giving up %lX\n", dev->status);<br>634:                return ret;<br>635:       }<br>636:         descriptor = (struct usb_hub_descriptor *)buffer;<br>637: <br>638:  length = min_t(int, descriptor->bLength,<br>639:                      sizeof(struct usb_hub_descriptor));<br>640: <br>641:      ret = usb_get_hub_descriptor(dev, buffer, length);<br>642:        if (ret < 0) {<br>643:                 debug("usb_hub_configure: failed to get hub " \<br>644:                       "descriptor 2nd giving up %lX\n", dev->status);<br>645:             return ret;<br>646:       }<br>647:         memcpy((unsigned char *)&hub->desc, buffer, length);<br>648:       /* adjust 16bit values */<br>649:         put_unaligned(le16_to_cpu(get_unaligned(<br>650:                  &descriptor->wHubCharacteristics)),<br>651:                        &hub->desc.wHubCharacteristics);<br>652:   /* set the bitmap */<br>653:      bitmap = (unsigned char *)&hub->desc.u.hs.DeviceRemovable[0];<br>654:      /* devices not removable by default */<br>655:    memset(bitmap, 0xff, (USB_MAXCHILDREN+1+7)/8);<br>656:    bitmap = (unsigned char *)&hub->desc.u.hs.PortPowerCtrlMask[0];<br>657:    memset(bitmap, 0xff, (USB_MAXCHILDREN+1+7)/8); /* PowerMask = 1B */<br>658: <br>659:        for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++)<br>660:             hub->desc.u.hs.DeviceRemovable[i] =<br>661:                    descriptor->u.hs.DeviceRemovable[i];<br>662: <br>663:    for (i = 0; i < ((hub->desc.bNbrPorts + 1 + 7)/8); i++)<br>664:             hub->desc.u.hs.PortPowerCtrlMask[i] =<br>665:                  descriptor->u.hs.PortPowerCtrlMask[i];<br>666: <br>667:  dev->maxchild = descriptor->bNbrPorts;<br>668:      debug("%d ports detected\n", dev->maxchild);<br>669: <br>670:  hubCharacteristics = get_unaligned(&hub->desc.wHubCharacteristics);<br><br><br>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.<br><br>There are two problems in this function:<br><br>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.<br><br>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.<br><br>I've checked the master branch of the usb core driver for Linux and I saw there are more checks around this same value: <a href="https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L1411">https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L1411</a><br></div></div>