USB Device buffer overflow

[Fabio Estevam]

Hi Szymon,

[Adding Marek]

On Wed, Nov 16, 2022 at 8:56 PM Szymon Heidrich
<szymon.heidrich at gmail.com> wrote:
>
> Hello,
>
> Similar to CVE-2021-39685 affecting the Linux kernel U-Boot is vulnerable to a buffer overflow
> present in the USB Gadget stack. Handling of a control transfer request with wLength larger than
> USB_BUFSIZ (4096) may result in a buffer overflow.
>
> The buffer for USB control endpoint is allocated in the composite_bind function implemented in
> drivers/usb/gadget/composite.c. The buffer size is set to USB_BUFSIZ (4096) bytes.
>
> >       /* preallocate control response and buffer */
> >       cdev->req = usb_ep_alloc_request(gadget->ep0, GFP_KERNEL);
> >       if (!cdev->req)
> >               goto fail;
> >       cdev->req->buf = memalign(CONFIG_SYS_CACHELINE_SIZE, USB_BUFSIZ);
> >       if (!cdev->req->buf)
> >               goto fail;
> >       cdev->req->complete = composite_setup_complete;
> >       gadget->ep0->driver_data = cdev;
>
> In the composite_setup function data transfer phase is set up to the length of "value" bytes
> which in multiple cases may be controlled by an attacker (is set to wLength).
>
> >       if (value >= 0) {
> >               req->length = value;
> >               req->zero = value < w_length;
> >               value = usb_ep_queue(gadget->ep0, req, GFP_KERNEL);
> >               if (value < 0) {
> >                       debug("ep_queue --> %d\n", value);
> >                       req->status = 0;
> >                       composite_setup_complete(gadget->ep0, req);
> >               }
> >       }
>
> In example the OS descriptor handler may be forced to set value to w_length.
>
> > } else {
> >     /* "extended compatibility ID"s */
> >     count = count_ext_compat(os_desc_cfg);
> >     buf[8] = count;
> >     count *= 24; /* 24 B/ext compat desc */
> >     count += 16; /* header */
> >     put_unaligned_le32(count, buf);
> >     buf += 16;
> >     fill_ext_compat(os_desc_cfg, buf);
> >     value = w_length;
> > }
>
> Execution of this code path for wLength set to a value larger then USB_BUFSIZ will result
> in a buffer overflow. Since wLength is a double byte value it may have values up to 0xffff.
>
> Besides the common OS descriptor handler this issue may be exploited for some of the available
> gadgets e.g. f_dfu, f_sdp where "value" is derived from wLength.
>
> drivers/usb/gadget/f_dfu.c
> > static int handle_dnload(struct usb_gadget *gadget, u16 len)
> > {
> >       struct usb_composite_dev *cdev = get_gadget_data(gadget);
> >       struct usb_request *req = cdev->req;
> >       struct f_dfu *f_dfu = req->context;
> >
> >       if (len == 0)
> >               f_dfu->dfu_state = DFU_STATE_dfuMANIFEST_SYNC;
> >
> >       req->complete = dnload_request_complete;
> >
> >       return len;
> > }
>
> drivers/usb/gadget/f_sdp.c
> > if (req_type == USB_TYPE_CLASS) {
> >    int report = w_value & HID_REPORT_ID_MASK;
> >
> >     /* HID (SDP) request */
> >     switch (ctrl->bRequest) {
> >     case HID_REQ_SET_REPORT:
> >         switch (report) {
> >         case 1:
> >             value = SDP_COMMAND_LEN + 1;
> >             req->complete = sdp_rx_command_complete;
> >             sdp_func->ep_int_enable = false;
> >             break;
> >         case 2:
> >             value = len;
> >             req->complete = sdp_rx_data_complete;
> >             sdp_func->state = SDP_STATE_RX_FILE_DATA_BUSY;
> >             break;
> >         }
> >     }
> > }
>
> Please find attached a patch addressing this issue.
> Depending on request direction wLength larger than USB_BUFSIZ will result in either
> endpoint stall or value trim.

Thanks for the patch. Please submit it via git send-email instead of attachment.