usb: gadget: dfu: Fix the unchecked length field
authorVenkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
Thu, 3 Nov 2022 04:07:48 +0000 (09:37 +0530)
committerTom Rini <trini@konsulko.com>
Mon, 21 Nov 2022 14:23:00 +0000 (09:23 -0500)
DFU implementation does not bound the length field in USB
DFU download setup packets, and it does not verify that
the transfer direction. Fixing the length and transfer
direction.

CVE-2022-2347

Signed-off-by: Venkatesh Yadav Abbarapu <venkatesh.abbarapu@amd.com>
Reviewed-by: Marek Vasut <marex@denx.de>
drivers/usb/gadget/f_dfu.c

index e9340ff..33ef62f 100644 (file)
@@ -321,23 +321,29 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
        u16 len = le16_to_cpu(ctrl->wLength);
        int value = 0;
 
+       len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
        switch (ctrl->bRequest) {
        case USB_REQ_DFU_DNLOAD:
-               if (len == 0) {
-                       f_dfu->dfu_state = DFU_STATE_dfuERROR;
-                       value = RET_STALL;
-                       break;
+               if (ctrl->bRequestType == USB_DIR_OUT) {
+                       if (len == 0) {
+                               f_dfu->dfu_state = DFU_STATE_dfuERROR;
+                               value = RET_STALL;
+                               break;
+                       }
+                       f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
+                       f_dfu->blk_seq_num = w_value;
+                       value = handle_dnload(gadget, len);
                }
-               f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
-               f_dfu->blk_seq_num = w_value;
-               value = handle_dnload(gadget, len);
                break;
        case USB_REQ_DFU_UPLOAD:
-               f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
-               f_dfu->blk_seq_num = 0;
-               value = handle_upload(req, len);
-               if (value >= 0 && value < len)
-                       f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+               if (ctrl->bRequestType == USB_DIR_IN) {
+                       f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
+                       f_dfu->blk_seq_num = 0;
+                       value = handle_upload(req, len);
+                       if (value >= 0 && value < len)
+                               f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+               }
                break;
        case USB_REQ_DFU_ABORT:
                /* no zlp? */
@@ -426,11 +432,15 @@ static int state_dfu_dnload_idle(struct f_dfu *f_dfu,
        u16 len = le16_to_cpu(ctrl->wLength);
        int value = 0;
 
+       len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
        switch (ctrl->bRequest) {
        case USB_REQ_DFU_DNLOAD:
-               f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
-               f_dfu->blk_seq_num = w_value;
-               value = handle_dnload(gadget, len);
+               if (ctrl->bRequestType == USB_DIR_OUT) {
+                       f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
+                       f_dfu->blk_seq_num = w_value;
+                       value = handle_dnload(gadget, len);
+               }
                break;
        case USB_REQ_DFU_ABORT:
                f_dfu->dfu_state = DFU_STATE_dfuIDLE;
@@ -513,13 +523,17 @@ static int state_dfu_upload_idle(struct f_dfu *f_dfu,
        u16 len = le16_to_cpu(ctrl->wLength);
        int value = 0;
 
+       len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
        switch (ctrl->bRequest) {
        case USB_REQ_DFU_UPLOAD:
-               /* state transition if less data then requested */
-               f_dfu->blk_seq_num = w_value;
-               value = handle_upload(req, len);
-               if (value >= 0 && value < len)
-                       f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+               if (ctrl->bRequestType == USB_DIR_IN) {
+                       /* state transition if less data then requested */
+                       f_dfu->blk_seq_num = w_value;
+                       value = handle_upload(req, len);
+                       if (value >= 0 && value < len)
+                               f_dfu->dfu_state = DFU_STATE_dfuIDLE;
+               }
                break;
        case USB_REQ_DFU_ABORT:
                f_dfu->dfu_state = DFU_STATE_dfuIDLE;
@@ -595,6 +609,8 @@ dfu_handle(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
        int value = 0;
        u8 req_type = ctrl->bRequestType & USB_TYPE_MASK;
 
+       len = len > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : len;
+
        debug("w_value: 0x%x len: 0x%x\n", w_value, len);
        debug("req_type: 0x%x ctrl->bRequest: 0x%x f_dfu->dfu_state: 0x%x\n",
               req_type, ctrl->bRequest, f_dfu->dfu_state);
@@ -614,7 +630,7 @@ dfu_handle(struct usb_function *f, const struct usb_ctrlrequest *ctrl)
                value = dfu_state[f_dfu->dfu_state] (f_dfu, ctrl, gadget, req);
 
        if (value >= 0) {
-               req->length = value;
+               req->length = value > DFU_USB_BUFSIZ ? DFU_USB_BUFSIZ : value;
                req->zero = value < len;
                value = usb_ep_queue(gadget->ep0, req, 0);
                if (value < 0) {