fix(security): check array overflowdev

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

4 files changed, 78 insertions, 24 deletions

diff --git a/class/cdc/usbd_cdc_acm.c b/class/cdc/usbd_cdc_acm.c
index 0c911c6c..bd6009b8 100644
--- a/class/cdc/usbd_cdc_acm.c
+++ b/class/cdc/usbd_cdc_acm.c
@@ -39,7 +39,16 @@ static int cdc_acm_class_interface_request_handler(uint8_t busid, struct usb_set
             /*                                        4 - Space                            */
             /* 6      | bDataBits  |   1   | Number Data bits (5, 6, 7, 8 or 16).          */
             /*******************************************************************************/
-            memcpy(&line_coding, *data, 7);
+            if (*len != sizeof(struct cdc_line_coding)) {
+                return -1;
+            }
+            memcpy(&line_coding, *data, sizeof(struct cdc_line_coding));
+
+            if (line_coding.bParityType >= ARRAY_SIZE(parity_name) ||
+                line_coding.bCharFormat >= ARRAY_SIZE(stop_name)) {
+                return -1;
+            }
+
             USB_LOG_DBG("Set intf:%d linecoding <%d %d %s %s>\r\n",
                         intf_num,
                         (unsigned int)line_coding.dwDTERate,
@@ -62,9 +71,19 @@ static int cdc_acm_class_interface_request_handler(uint8_t busid, struct usb_set
             break;
 
         case CDC_REQUEST_GET_LINE_CODING:
+            if (*len != sizeof(struct cdc_line_coding)) {
+                return -1;
+            }
+
             usbd_cdc_acm_get_line_coding(busid, intf_num, &line_coding);
-            memcpy(*data, &line_coding, 7);
-            *len = 7;
+            memcpy(*data, &line_coding, sizeof(struct cdc_line_coding));
+
+            if (line_coding.bParityType >= ARRAY_SIZE(parity_name) ||
+                line_coding.bCharFormat >= ARRAY_SIZE(stop_name)) {
+                return -1;
+            }
+
+            *len = sizeof(struct cdc_line_coding);
             USB_LOG_DBG("Get intf:%d linecoding %d %d %d %d\r\n",
                         intf_num,
                         (unsigned int)line_coding.dwDTERate,
diff --git a/class/cdc/usbd_cdc_ecm.c b/class/cdc/usbd_cdc_ecm.c
index bb3071ee..ce0dbf39 100644
--- a/class/cdc/usbd_cdc_ecm.c
+++ b/class/cdc/usbd_cdc_ecm.c
@@ -235,8 +235,12 @@ struct pbuf *usbd_cdc_ecm_eth_rx(void)
         usbd_cdc_ecm_start_read(g_cdc_ecm_rx_buffer, CONFIG_CDC_ECM_ETH_MAX_SEGSZE);
         return NULL;
     }
-    usb_memcpy(p->payload, (uint8_t *)g_cdc_ecm_rx_buffer, g_cdc_ecm_rx_data_length);
-    p->len = g_cdc_ecm_rx_data_length;
+
+    if (pbuf_take(p, g_cdc_ecm_rx_buffer, g_cdc_ecm_rx_data_length) != ERR_OK) {
+        pbuf_free(p);
+        usbd_cdc_ecm_start_read(g_cdc_ecm_rx_buffer, CONFIG_CDC_ECM_ETH_MAX_SEGSZE);
+        return NULL;
+    }
 
     USB_LOG_DBG("rxlen:%d\r\n", g_cdc_ecm_rx_data_length);
     usbd_cdc_ecm_start_read(g_cdc_ecm_rx_buffer, CONFIG_CDC_ECM_ETH_MAX_SEGSZE);
@@ -257,7 +261,7 @@ int usbd_cdc_ecm_eth_tx(struct pbuf *p)
     }
 
     if (p->tot_len > sizeof(g_cdc_ecm_tx_buffer)) {
-        p->tot_len = sizeof(g_cdc_ecm_tx_buffer);
+        return -USB_ERR_NOMEM;
     }
 
     buffer = g_cdc_ecm_tx_buffer;
diff --git a/class/cdc/usbh_cdc_ecm.c b/class/cdc/usbh_cdc_ecm.c
index 35b92e2a..248b942b 100644
--- a/class/cdc/usbh_cdc_ecm.c
+++ b/class/cdc/usbh_cdc_ecm.c
@@ -92,15 +92,21 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
     hport->config.intf[intf].priv = cdc_ecm_class;
     hport->config.intf[intf + 1].priv = NULL;
 
+    if (hport->config.intf[intf + 1].altsetting[0].intf_desc.bInterfaceClass != USB_DEVICE_CLASS_CDC_DATA) {
+        return -USB_ERR_INVAL;
+    }
+
     p = hport->raw_config_desc;
-    while (p[DESC_bLength]) {
+    while (desc_len < hport->config.config_desc.wTotalLength) {
         switch (p[DESC_bDescriptorType]) {
             case USB_DESCRIPTOR_TYPE_INTERFACE:
                 cur_iface = p[INTF_DESC_bInterfaceNumber];
                 //cur_alt_setting = p[INTF_DESC_bAlternateSetting];
                 break;
             case CDC_CS_INTERFACE:
-                if ((cur_iface == cdc_ecm_class->ctrl_intf) && p[DESC_bDescriptorSubType] == CDC_FUNC_DESC_ETHERNET_NETWORKING) {
+                if ((cur_iface == cdc_ecm_class->ctrl_intf) &&
+                    (p[DESC_bDescriptorSubType] == CDC_FUNC_DESC_ETHERNET_NETWORKING) &&
+                    (p[DESC_bLength] == sizeof(struct cdc_eth_descriptor))) {
                     struct cdc_eth_descriptor *desc = (struct cdc_eth_descriptor *)p;
                     mac_str_idx = desc->iMACAddress;
                     cdc_ecm_class->max_segment_size = desc->wMaxSegmentSize;
@@ -114,10 +120,6 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
         /* 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:
@@ -300,6 +302,10 @@ int usbh_cdc_ecm_eth_output(uint32_t buflen)
         return -USB_ERR_NOTCONN;
     }
 
+    if (buflen > sizeof(g_cdc_ecm_tx_buffer)) {
+        return -USB_ERR_NOMEM;
+    }
+
     USB_LOG_DBG("txlen:%d\r\n", buflen);
 
     usbh_bulk_urb_fill(&g_cdc_ecm_class.bulkout_urb, g_cdc_ecm_class.hport, g_cdc_ecm_class.bulkout, g_cdc_ecm_tx_buffer, buflen, USB_OSAL_WAITING_FOREVER, NULL, NULL);
diff --git a/class/cdc/usbh_cdc_ncm.c b/class/cdc/usbh_cdc_ncm.c
index 83310e09..60c70fa8 100644
--- a/class/cdc/usbh_cdc_ncm.c
+++ b/class/cdc/usbh_cdc_ncm.c
@@ -124,14 +124,16 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
     hport->config.intf[intf + 1].priv = NULL;
 
     p = hport->raw_config_desc;
-    while (p[DESC_bLength]) {
+    while (desc_len < hport->config.config_desc.wTotalLength) {
         switch (p[DESC_bDescriptorType]) {
             case USB_DESCRIPTOR_TYPE_INTERFACE:
                 cur_iface = p[INTF_DESC_bInterfaceNumber];
                 //cur_alt_setting = p[INTF_DESC_bAlternateSetting];
                 break;
             case CDC_CS_INTERFACE:
-                if ((cur_iface == cdc_ncm_class->ctrl_intf) && p[DESC_bDescriptorSubType] == CDC_FUNC_DESC_ETHERNET_NETWORKING) {
+                if ((cur_iface == cdc_ncm_class->ctrl_intf) &&
+                    (p[DESC_bDescriptorSubType] == CDC_FUNC_DESC_ETHERNET_NETWORKING) &&
+                    (p[DESC_bLength] == sizeof(struct cdc_eth_descriptor))) {
                     struct cdc_eth_descriptor *desc = (struct cdc_eth_descriptor *)p;
                     mac_str_idx = desc->iMACAddress;
                     cdc_ncm_class->max_segment_size = desc->wMaxSegmentSize;
@@ -145,10 +147,6 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
         /* 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:
@@ -289,13 +287,16 @@ void usbh_cdc_ncm_rx_thread(CONFIG_USB_OSAL_THREAD_SET_ARGV)
         }
 
         g_cdc_ncm_rx_length = g_cdc_ncm_class.bulkin_urb.actual_length;
-        if(g_cdc_ncm_rx_length == 0) {
+        if (g_cdc_ncm_rx_length == 0) {
             continue;
         }
 
         if (g_cdc_ncm_rx_length < CONFIG_USBHOST_CDC_NCM_ETH_MAX_RX_SIZE) {
             USB_LOG_DBG("rxlen:%d\r\n", g_cdc_ncm_rx_length);
 
+            if (g_cdc_ncm_rx_length < sizeof(struct cdc_ncm_nth16)) {
+                continue;
+            }
             struct cdc_ncm_nth16 *nth16 = (struct cdc_ncm_nth16 *)&g_cdc_ncm_rx_buffer[0];
             if ((nth16->dwSignature != CDC_NCM_NTH16_SIGNATURE) ||
                 (nth16->wHeaderLength != 12) ||
@@ -305,6 +306,12 @@ void usbh_cdc_ncm_rx_thread(CONFIG_USB_OSAL_THREAD_SET_ARGV)
                 continue;
             }
 
+            if (nth16->wNdpIndex < sizeof(struct cdc_ncm_nth16) ||
+                (nth16->wNdpIndex & 3) ||
+                nth16->wNdpIndex > g_cdc_ncm_rx_length - sizeof(struct cdc_ncm_ndp16)) {
+                USB_LOG_ERR("invalid rx ndp16 index\r\n");
+                continue;
+            }
             struct cdc_ncm_ndp16 *ndp16 = (struct cdc_ncm_ndp16 *)&g_cdc_ncm_rx_buffer[nth16->wNdpIndex];
             if ((ndp16->dwSignature != CDC_NCM_NDP16_SIGNATURE_NCM0) && (ndp16->dwSignature != CDC_NCM_NDP16_SIGNATURE_NCM1)) {
                 USB_LOG_ERR("invalid rx ndp16\r\n");
@@ -312,17 +319,31 @@ void usbh_cdc_ncm_rx_thread(CONFIG_USB_OSAL_THREAD_SET_ARGV)
                 continue;
             }
 
+            if (ndp16->wLength < sizeof(struct cdc_ncm_ndp16) + sizeof(struct cdc_ncm_ndp16_datagram) ||
+                (ndp16->wLength & 3) ||
+                ndp16->wLength > g_cdc_ncm_rx_length - nth16->wNdpIndex) {
+                USB_LOG_ERR("invalid rx ndp16 length\r\n");
+                continue;
+            }
             uint16_t datagram_num = (ndp16->wLength - 8) / 4;
 
             USB_LOG_DBG("datagram num:%02x\r\n", datagram_num);
             for (uint16_t i = 0; i < datagram_num; i++) {
                 struct cdc_ncm_ndp16_datagram *ndp16_datagram = (struct cdc_ncm_ndp16_datagram *)&g_cdc_ncm_rx_buffer[nth16->wNdpIndex + 8 + 4 * i];
-                if (ndp16_datagram->wDatagramIndex && ndp16_datagram->wDatagramLength) {
-                    USB_LOG_DBG("ndp16_datagram index:%02x, length:%02x\r\n", ndp16_datagram->wDatagramIndex, ndp16_datagram->wDatagramLength);
-
-                    uint8_t *buf = (uint8_t *)&g_cdc_ncm_rx_buffer[ndp16_datagram->wDatagramIndex];
-                    usbh_cdc_ncm_eth_input(buf, ndp16_datagram->wDatagramLength);
+                if (!ndp16_datagram->wDatagramIndex && !ndp16_datagram->wDatagramLength) {
+                    break;
+                }
+                if (ndp16_datagram->wDatagramIndex < sizeof(struct cdc_ncm_nth16) ||
+                    ndp16_datagram->wDatagramIndex > g_cdc_ncm_rx_length ||
+                    !ndp16_datagram->wDatagramLength ||
+                    ndp16_datagram->wDatagramLength > g_cdc_ncm_rx_length - ndp16_datagram->wDatagramIndex) {
+                    USB_LOG_ERR("invalid rx datagram range\r\n");
+                    break;
                 }
+                USB_LOG_DBG("ndp16_datagram index:%02x, length:%02x\r\n", ndp16_datagram->wDatagramIndex, ndp16_datagram->wDatagramLength);
+
+                uint8_t *buf = (uint8_t *)&g_cdc_ncm_rx_buffer[ndp16_datagram->wDatagramIndex];
+                usbh_cdc_ncm_eth_input(buf, ndp16_datagram->wDatagramLength);
             }
 
             g_cdc_ncm_rx_length = 0;
@@ -350,6 +371,10 @@ int usbh_cdc_ncm_eth_output(uint32_t buflen)
         return -USB_ERR_NOTCONN;
     }
 
+    if ((32U + USB_ALIGN_UP(buflen, 4)) > sizeof(g_cdc_ncm_tx_buffer)) {
+        return -USB_ERR_NOMEM;
+    }
+
     struct cdc_ncm_nth16 *nth16 = (struct cdc_ncm_nth16 *)&g_cdc_ncm_tx_buffer[0];
 
     nth16->dwSignature = CDC_NCM_NTH16_SIGNATURE;