[PATCH] usb: gadget: dfu: Fix the unchecked length field

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>
Note (<dleidert>: I'm not sure if this patch should be applied as well:
https://source.denx.de/u-boot/u-boot/-/commit/86b6a38863bebb70a65a53f93a1ffafc4a472169

It is not related to the issue, though.

Reviewed-By: Daniel Leidert <dleidert@debian.org>
Origin: https://source.denx.de/u-boot/u-boot/-/commit/fbce985e28eaca3af82afecc11961aadaf971a7e
Bug: https://www.openwall.com/lists/oss-security/2022/07/08/2
Bug-Debian: https://bugs.debian.org/1014959
Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2022-2347
Bug-Freexian-Security: https://deb.freexian.com/extended-lts/tracker/CVE-2022-2347

Gbp-Pq: Name CVE-2022-2347.patch

diff --git a/drivers/usb/gadget/f_dfu.c b/drivers/usb/gadget/f_dfu.c
index 4bedc7d3a190ea7c6d1b54f810fecb2c62fe420f..ff1dd2b5d794c9c695f0c6f206c4a3b41b71ca6c 100644 (file)
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -321,21 +321,27 @@ 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 (ctrl->bRequestType == USB_DIR_IN) {
+                       f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
+                       f_dfu->blk_seq_num = 0;
+                       value = handle_upload(req, len);
+               }
                break;
        case USB_REQ_DFU_ABORT:
                /* no zlp? */
@@ -424,11 +430,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;
@@ -511,13 +521,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;
@@ -593,6 +607,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);
@@ -612,7 +628,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) {