fix(security): check desc len overflow

Signed-off-by: sakumisu <[email protected]>

7 files changed, 59 insertions, 13 deletions

diff --git a/class/audio/usbh_audio.c b/class/audio/usbh_audio.c
index 06f68dcf..ee0a3349 100644
--- a/class/audio/usbh_audio.c
+++ b/class/audio/usbh_audio.c
@@ -628,6 +628,7 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
     uint8_t cur_alt_setting = 0;
     const uint8_t *p;
     const uint8_t *p_ac_desc = NULL;
+    uint32_t desc_len = 0;
 
     struct usbh_audio *audio_class = usbh_audio_class_alloc();
     if (audio_class == NULL) {
@@ -698,7 +699,7 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
                             USB_ASSERT(cur_alt_setting < CONFIG_USBHOST_MAX_INTF_ALTSETTINGS);
 
                             audio_class->as_msg_table[cur_iface - audio_class->ctrl_intf - 1].bNrChannels = desc->bNrChannels;
-                            memcpy(&audio_class->as_msg_table[cur_iface - audio_class->ctrl_intf - 1].as_format[cur_alt_setting], desc, desc->bLength);
+                            memcpy(&audio_class->as_msg_table[cur_iface - audio_class->ctrl_intf - 1].as_format[cur_alt_setting], desc, AUDIO_SIZEOF_FORMAT_TYPE_DESC(1));
                         } break;
                         default:
                             break;
@@ -718,7 +719,12 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
                 break;
         }
         /* skip to next descriptor */
+        desc_len += p[DESC_bLength];
         p += p[DESC_bLength];
+
+        if (desc_len > hport->config.config_desc.wTotalLength) {
+            return -USB_ERR_INVAL;
+        }
     }
 
     if (p_ac_desc == NULL) {
diff --git a/class/cdc/usbh_cdc_ecm.c b/class/cdc/usbh_cdc_ecm.c
index 0c64ff11..6fef9b20 100644
--- a/class/cdc/usbh_cdc_ecm.c
+++ b/class/cdc/usbh_cdc_ecm.c
@@ -79,6 +79,7 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
     uint8_t *p;
     uint8_t cur_iface = 0xff;
     uint8_t mac_str_idx = 0xff;
+    uint32_t desc_len = 0;
 
     struct usbh_cdc_ecm *cdc_ecm_class = &g_cdc_ecm_class;
 
@@ -111,7 +112,12 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
                 break;
         }
         /* skip to next descriptor */
+        desc_len += p[DESC_bLength];
         p += p[DESC_bLength];
+
+        if (desc_len > hport->config.config_desc.wTotalLength) {
+            return -USB_ERR_INVAL;
+        }
     }
 
 get_mac:
diff --git a/class/cdc/usbh_cdc_ncm.c b/class/cdc/usbh_cdc_ncm.c
index fa2ccc0c..225a7fcd 100644
--- a/class/cdc/usbh_cdc_ncm.c
+++ b/class/cdc/usbh_cdc_ncm.c
@@ -106,6 +106,7 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
     uint8_t *p;
     uint8_t cur_iface = 0xff;
     uint8_t mac_str_idx = 0xff;
+    uint32_t desc_len = 0;
 
     struct usbh_cdc_ncm *cdc_ncm_class = &g_cdc_ncm_class;
 
@@ -138,7 +139,12 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
                 break;
         }
         /* skip to next descriptor */
+        desc_len += p[DESC_bLength];
         p += p[DESC_bLength];
+
+        if (desc_len > hport->config.config_desc.wTotalLength) {
+            return -USB_ERR_INVAL;
+        }
     }
 
 get_mac:
diff --git a/class/hid/usbh_hid.c b/class/hid/usbh_hid.c
index 16e15adb..3039f5aa 100644
--- a/class/hid/usbh_hid.c
+++ b/class/hid/usbh_hid.c
@@ -194,6 +194,7 @@ int usbh_hid_connect(struct usbh_hubport *hport, uint8_t intf)
     uint8_t cur_iface = 0xff;
     uint8_t *p;
     bool found = false;
+    uint32_t desc_len = 0;
 
     struct usbh_hid *hid_class = usbh_hid_class_alloc();
     if (hid_class == NULL) {
@@ -234,7 +235,12 @@ int usbh_hid_connect(struct usbh_hubport *hport, uint8_t intf)
                 break;
         }
         /* skip to next descriptor */
+        desc_len += p[DESC_bLength];
         p += p[DESC_bLength];
+
+        if (desc_len > hport->config.config_desc.wTotalLength) {
+            return -USB_ERR_INVAL;
+        }
     }
 
     if (found == false) {
@@ -318,6 +324,10 @@ int usbh_hid_parse_report_descriptor(const uint8_t *report_data, uint32_t report
         if (itemsize == 3) /* HID spec: 6.2.2.2 - Short Items */
             itemsize = 4;
 
+        if((itemsize + i + 1) > report_size) {
+            goto err;
+        }
+
         itemval = hid_get_itemval(report_data, i, itemsize);
 
         USB_LOG_DBG("itemtype 0x%02x, itemtag 0x%02x, itemsize %d, itemval 0x%08x\r\n",
diff --git a/class/video/usbd_video.c b/class/video/usbd_video.c
index 3ed6ff88..071e8936 100644
--- a/class/video/usbd_video.c
+++ b/class/video/usbd_video.c
@@ -561,10 +561,10 @@ static int usbd_video_stream_request_handler(uint8_t busid, struct usb_setup_pac
         case VIDEO_VS_PROBE_CONTROL:
             switch (setup->bRequest) {
                 case VIDEO_REQUEST_SET_CUR:
-                    //memcpy((uint8_t *)&g_usbd_video[busid].probe, *data, setup->wLength);
+                    //memcpy((uint8_t *)&g_usbd_video[busid].probe, *data, sizeof(struct video_probe_and_commit_controls));
                     break;
                 case VIDEO_REQUEST_GET_CUR:
-                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, setup->wLength);
+                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, sizeof(struct video_probe_and_commit_controls));
                     *len = sizeof(struct video_probe_and_commit_controls);
                     break;
 
@@ -572,7 +572,7 @@ static int usbd_video_stream_request_handler(uint8_t busid, struct usb_setup_pac
                 case VIDEO_REQUEST_GET_MAX:
                 case VIDEO_REQUEST_GET_RES:
                 case VIDEO_REQUEST_GET_DEF:
-                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, setup->wLength);
+                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, sizeof(struct video_probe_and_commit_controls));
                     *len = sizeof(struct video_probe_and_commit_controls);
                     break;
                 case VIDEO_REQUEST_GET_LEN:
@@ -592,17 +592,17 @@ static int usbd_video_stream_request_handler(uint8_t busid, struct usb_setup_pac
         case VIDEO_VS_COMMIT_CONTROL:
             switch (setup->bRequest) {
                 case VIDEO_REQUEST_SET_CUR:
-                    //memcpy((uint8_t *)&g_usbd_video[busid].commit, *data, setup->wLength);
+                    //memcpy((uint8_t *)&g_usbd_video[busid].commit, *data, sizeof(struct video_probe_and_commit_controls));
                     break;
                 case VIDEO_REQUEST_GET_CUR:
-                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, setup->wLength);
+                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, sizeof(struct video_probe_and_commit_controls));
                     *len = sizeof(struct video_probe_and_commit_controls);
                     break;
                 case VIDEO_REQUEST_GET_MIN:
                 case VIDEO_REQUEST_GET_MAX:
                 case VIDEO_REQUEST_GET_RES:
                 case VIDEO_REQUEST_GET_DEF:
-                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, setup->wLength);
+                    memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, sizeof(struct video_probe_and_commit_controls));
                     *len = sizeof(struct video_probe_and_commit_controls);
                     break;
 
diff --git a/class/video/usbh_video.c b/class/video/usbh_video.c
index 8b7863cc..8ecd252d 100644
--- a/class/video/usbh_video.c
+++ b/class/video/usbh_video.c
@@ -382,6 +382,7 @@ static int usbh_video_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
     uint8_t frame_index = 0xff;
     uint8_t format_index = 0xff;
     uint8_t num_of_frames = 0xff;
+    uint32_t desc_len = 0;
     uint8_t *p;
 
     struct usbh_video *video_class = usbh_video_class_alloc();
@@ -489,7 +490,12 @@ static int usbh_video_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
                 break;
         }
         /* skip to next descriptor */
+        desc_len += p[DESC_bLength];
         p += p[DESC_bLength];
+
+        if (desc_len > hport->config.config_desc.wTotalLength) {
+            return -USB_ERR_INVAL;
+        }
     }
 
     usbh_video_list_info(video_class);
diff --git a/core/usbh_core.c b/core/usbh_core.c
index 5828a498..b7f762ea 100644
--- a/core/usbh_core.c
+++ b/core/usbh_core.c
@@ -228,6 +228,9 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
         while (p[DESC_bLength]) {
             switch (p[DESC_bDescriptorType]) {
                 case USB_DESCRIPTOR_TYPE_INTERFACE:
+                    if (p[DESC_bLength] < USB_SIZEOF_INTERFACE_DESC) {
+                        return -USB_ERR_INVAL;
+                    }
                     intf_desc = (struct usb_interface_descriptor *)p;
                     cur_iface = intf_desc->bInterfaceNumber;
                     cur_alt_setting = intf_desc->bAlternateSetting;
@@ -265,6 +268,9 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
                     hport->config.intf[cur_iface].altsetting_num = cur_alt_setting + 1;
                     break;
                 case USB_DESCRIPTOR_TYPE_ENDPOINT:
+                    if (p[DESC_bLength] < USB_SIZEOF_ENDPOINT_DESC) {
+                        return -USB_ERR_INVAL;
+                    }
                     ep_desc = (struct usb_endpoint_descriptor *)p;
                     memcpy(&hport->config.intf[cur_iface].altsetting[cur_alt_setting].ep[cur_ep].ep_desc, ep_desc, 7);
                     cur_ep++;
@@ -277,10 +283,15 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
             desc_len += p[DESC_bLength];
             p += p[DESC_bLength];
 
-            if(desc_len > length) {
-                return -USB_ERR_NOMEM;
+            if (desc_len > length) {
+                return -USB_ERR_INVAL;
             }
         }
+
+        if (desc_len != length) {
+            USB_LOG_ERR("Config descriptor length mismatch, expect %d, actual %d\r\n", length, desc_len);
+            return -USB_ERR_INVAL;
+        }
     }
     return 0;
 }
@@ -447,7 +458,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
     /* Read the full size of the configuration data */
     uint16_t wTotalLength = ((struct usb_configuration_descriptor *)ep0_request_buffer[hport->bus->busid])->wTotalLength;
 
-    if (wTotalLength > CONFIG_USBHOST_REQUEST_BUFFER_LEN) {
+    if (wTotalLength >= CONFIG_USBHOST_REQUEST_BUFFER_LEN) {
         ret = -USB_ERR_NOMEM;
         USB_LOG_ERR("wTotalLength %d is overflow, default is %d\r\n", wTotalLength, (unsigned int)CONFIG_USBHOST_REQUEST_BUFFER_LEN);
         goto errout;
@@ -465,6 +476,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
         goto errout;
     }
 
+    ep0_request_buffer[hport->bus->busid][wTotalLength] = '\0';
     ret = parse_config_descriptor(hport, (struct usb_configuration_descriptor *)ep0_request_buffer[hport->bus->busid], wTotalLength);
     if (ret < 0) {
         USB_LOG_ERR("Parse config descriptor fail\r\n");
@@ -803,9 +815,7 @@ static struct usbh_hubport *usbh_list_all_hubport(struct usbh_hub *hub, uint8_t
     struct usbh_hubport *hport;
     struct usbh_hub *hub_next;
 
-    if ((hub_index > hub->index) || (hub_port > hub->nports)) {
-        return NULL;
-    }
+    USB_ASSERT((hub_index > 0) && (hub_port > 0) && (hub_index <= hub->index) && (hub_port <= hub->nports));
 
     if (hub->index == hub_index) {
         hport = &hub->child[hub_port - 1];
@@ -869,6 +879,8 @@ struct usbh_hubport *usbh_find_hubport(uint8_t busid, uint8_t hub_index, uint8_t
     struct usbh_hubport *hport;
     size_t flags;
 
+    USB_ASSERT_MSG(busid < CONFIG_USBHOST_MAX_BUS, "bus overflow\r\n");
+
     flags = usb_osal_enter_critical_section();
 
     bus = &g_usbhost_bus[busid];