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.
Best regards,
SzymonFrom 1723d976ab0387be0d91b177011559cb07a66e03 Mon Sep 17 00:00:00 2001
From: Szymon Heidrich <szymon.heidr...@gmail.com>
Date: Wed, 9 Nov 2022 17:55:34 +0100
Subject: [PATCH] Prevent buffer overflow on USB control endpoint
Assure that the control endpoint buffer of size USB_BUFSIZ (4096)
can not be overflown during handling of USB control transfer
requests with wLength greater than USB_BUFSIZ.
Signed-off-by: Szymon Heidrich <szymon.heidr...@gmail.com>
---
drivers/usb/gadget/composite.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 2a309e62..cb89f6dc 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1019,6 +1019,17 @@ composite_setup(struct usb_gadget *gadget, const struct
usb_ctrlrequest *ctrl)
u8 endp;
struct usb_configuration *c;
+ if (w_length > USB_BUFSIZ) {
+ if (ctrl->bRequestType & USB_DIR_IN) {
+ /* Cast away the const, we are going to overwrite on
purpose. */
+ __le16 *temp = (__le16 *)&ctrl->wLength;
+ *temp = cpu_to_le16(USB_BUFSIZ);
+ w_length = USB_BUFSIZ;
+ } else {
+ goto done;
+ }
+ }
+
/*
* partial re-init of the response message; the function or the
* gadget might need to intercept e.g. a control-OUT completion
--
2.38.1
