Skip to content

Commit bce3346

Browse files
sakumisusakumisu
committed
fix(security): check desc len overflow
Signed-off-by: sakumisu <1203593632@qq.com>
1 parent 5e9d0e6 commit bce3346

7 files changed

Lines changed: 59 additions & 13 deletions

File tree

‎class/audio/usbh_audio.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,6 +628,7 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
628628
uint8_t cur_alt_setting = 0;
629629
const uint8_t *p;
630630
const uint8_t *p_ac_desc = NULL;
631+
uint32_t desc_len = 0;
631632

632633
struct usbh_audio *audio_class = usbh_audio_class_alloc();
633634
if (audio_class == NULL) {
@@ -698,7 +699,7 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
698699
USB_ASSERT(cur_alt_setting < CONFIG_USBHOST_MAX_INTF_ALTSETTINGS);
699700

700701
audio_class->as_msg_table[cur_iface - audio_class->ctrl_intf - 1].bNrChannels = desc->bNrChannels;
701-
memcpy(&audio_class->as_msg_table[cur_iface - audio_class->ctrl_intf - 1].as_format[cur_alt_setting], desc, desc->bLength);
702+
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));
702703
} break;
703704
default:
704705
break;
@@ -718,7 +719,12 @@ static int usbh_audio_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
718719
break;
719720
}
720721
/* skip to next descriptor */
722+
desc_len += p[DESC_bLength];
721723
p += p[DESC_bLength];
724+
725+
if (desc_len > hport->config.config_desc.wTotalLength) {
726+
return -USB_ERR_INVAL;
727+
}
722728
}
723729

724730
if (p_ac_desc == NULL) {

‎class/cdc/usbh_cdc_ecm.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
7979
uint8_t *p;
8080
uint8_t cur_iface = 0xff;
8181
uint8_t mac_str_idx = 0xff;
82+
uint32_t desc_len = 0;
8283

8384
struct usbh_cdc_ecm *cdc_ecm_class = &g_cdc_ecm_class;
8485

@@ -111,7 +112,12 @@ static int usbh_cdc_ecm_connect(struct usbh_hubport *hport, uint8_t intf)
111112
break;
112113
}
113114
/* skip to next descriptor */
115+
desc_len += p[DESC_bLength];
114116
p += p[DESC_bLength];
117+
118+
if (desc_len > hport->config.config_desc.wTotalLength) {
119+
return -USB_ERR_INVAL;
120+
}
115121
}
116122

117123
get_mac:

‎class/cdc/usbh_cdc_ncm.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
106106
uint8_t *p;
107107
uint8_t cur_iface = 0xff;
108108
uint8_t mac_str_idx = 0xff;
109+
uint32_t desc_len = 0;
109110

110111
struct usbh_cdc_ncm *cdc_ncm_class = &g_cdc_ncm_class;
111112

@@ -138,7 +139,12 @@ static int usbh_cdc_ncm_connect(struct usbh_hubport *hport, uint8_t intf)
138139
break;
139140
}
140141
/* skip to next descriptor */
142+
desc_len += p[DESC_bLength];
141143
p += p[DESC_bLength];
144+
145+
if (desc_len > hport->config.config_desc.wTotalLength) {
146+
return -USB_ERR_INVAL;
147+
}
142148
}
143149

144150
get_mac:

‎class/hid/usbh_hid.c‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ int usbh_hid_connect(struct usbh_hubport *hport, uint8_t intf)
194194
uint8_t cur_iface = 0xff;
195195
uint8_t *p;
196196
bool found = false;
197+
uint32_t desc_len = 0;
197198

198199
struct usbh_hid *hid_class = usbh_hid_class_alloc();
199200
if (hid_class == NULL) {
@@ -234,7 +235,12 @@ int usbh_hid_connect(struct usbh_hubport *hport, uint8_t intf)
234235
break;
235236
}
236237
/* skip to next descriptor */
238+
desc_len += p[DESC_bLength];
237239
p += p[DESC_bLength];
240+
241+
if (desc_len > hport->config.config_desc.wTotalLength) {
242+
return -USB_ERR_INVAL;
243+
}
238244
}
239245

240246
if (found == false) {
@@ -318,6 +324,10 @@ int usbh_hid_parse_report_descriptor(const uint8_t *report_data, uint32_t report
318324
if (itemsize == 3) /* HID spec: 6.2.2.2 - Short Items */
319325
itemsize = 4;
320326

327+
if((itemsize + i + 1) > report_size) {
328+
goto err;
329+
}
330+
321331
itemval = hid_get_itemval(report_data, i, itemsize);
322332

323333
USB_LOG_DBG("itemtype 0x%02x, itemtag 0x%02x, itemsize %d, itemval 0x%08x\r\n",

‎class/video/usbd_video.c‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -561,18 +561,18 @@ static int usbd_video_stream_request_handler(uint8_t busid, struct usb_setup_pac
561561
case VIDEO_VS_PROBE_CONTROL:
562562
switch (setup->bRequest) {
563563
case VIDEO_REQUEST_SET_CUR:
564-
//memcpy((uint8_t *)&g_usbd_video[busid].probe, *data, setup->wLength);
564+
//memcpy((uint8_t *)&g_usbd_video[busid].probe, *data, sizeof(struct video_probe_and_commit_controls));
565565
break;
566566
case VIDEO_REQUEST_GET_CUR:
567-
memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, setup->wLength);
567+
memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, sizeof(struct video_probe_and_commit_controls));
568568
*len = sizeof(struct video_probe_and_commit_controls);
569569
break;
570570

571571
case VIDEO_REQUEST_GET_MIN:
572572
case VIDEO_REQUEST_GET_MAX:
573573
case VIDEO_REQUEST_GET_RES:
574574
case VIDEO_REQUEST_GET_DEF:
575-
memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, setup->wLength);
575+
memcpy(*data, (uint8_t *)&g_usbd_video[busid].probe, sizeof(struct video_probe_and_commit_controls));
576576
*len = sizeof(struct video_probe_and_commit_controls);
577577
break;
578578
case VIDEO_REQUEST_GET_LEN:
@@ -592,17 +592,17 @@ static int usbd_video_stream_request_handler(uint8_t busid, struct usb_setup_pac
592592
case VIDEO_VS_COMMIT_CONTROL:
593593
switch (setup->bRequest) {
594594
case VIDEO_REQUEST_SET_CUR:
595-
//memcpy((uint8_t *)&g_usbd_video[busid].commit, *data, setup->wLength);
595+
//memcpy((uint8_t *)&g_usbd_video[busid].commit, *data, sizeof(struct video_probe_and_commit_controls));
596596
break;
597597
case VIDEO_REQUEST_GET_CUR:
598-
memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, setup->wLength);
598+
memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, sizeof(struct video_probe_and_commit_controls));
599599
*len = sizeof(struct video_probe_and_commit_controls);
600600
break;
601601
case VIDEO_REQUEST_GET_MIN:
602602
case VIDEO_REQUEST_GET_MAX:
603603
case VIDEO_REQUEST_GET_RES:
604604
case VIDEO_REQUEST_GET_DEF:
605-
memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, setup->wLength);
605+
memcpy(*data, (uint8_t *)&g_usbd_video[busid].commit, sizeof(struct video_probe_and_commit_controls));
606606
*len = sizeof(struct video_probe_and_commit_controls);
607607
break;
608608

‎class/video/usbh_video.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,7 @@ static int usbh_video_ctrl_connect(struct usbh_hubport *hport, uint8_t intf)
382382
uint8_t frame_index = 0xff;
383383
uint8_t format_index = 0xff;
384384
uint8_t num_of_frames = 0xff;
385+
uint32_t desc_len = 0;
385386
uint8_t *p;
386387

387388
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)
489490
break;
490491
}
491492
/* skip to next descriptor */
493+
desc_len += p[DESC_bLength];
492494
p += p[DESC_bLength];
495+
496+
if (desc_len > hport->config.config_desc.wTotalLength) {
497+
return -USB_ERR_INVAL;
498+
}
493499
}
494500

495501
usbh_video_list_info(video_class);

‎core/usbh_core.c‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,9 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
228228
while (p[DESC_bLength]) {
229229
switch (p[DESC_bDescriptorType]) {
230230
case USB_DESCRIPTOR_TYPE_INTERFACE:
231+
if (p[DESC_bLength] < USB_SIZEOF_INTERFACE_DESC) {
232+
return -USB_ERR_INVAL;
233+
}
231234
intf_desc = (struct usb_interface_descriptor *)p;
232235
cur_iface = intf_desc->bInterfaceNumber;
233236
cur_alt_setting = intf_desc->bAlternateSetting;
@@ -265,6 +268,9 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
265268
hport->config.intf[cur_iface].altsetting_num = cur_alt_setting + 1;
266269
break;
267270
case USB_DESCRIPTOR_TYPE_ENDPOINT:
271+
if (p[DESC_bLength] < USB_SIZEOF_ENDPOINT_DESC) {
272+
return -USB_ERR_INVAL;
273+
}
268274
ep_desc = (struct usb_endpoint_descriptor *)p;
269275
memcpy(&hport->config.intf[cur_iface].altsetting[cur_alt_setting].ep[cur_ep].ep_desc, ep_desc, 7);
270276
cur_ep++;
@@ -277,10 +283,15 @@ static int parse_config_descriptor(struct usbh_hubport *hport, struct usb_config
277283
desc_len += p[DESC_bLength];
278284
p += p[DESC_bLength];
279285

280-
if(desc_len > length) {
281-
return -USB_ERR_NOMEM;
286+
if (desc_len > length) {
287+
return -USB_ERR_INVAL;
282288
}
283289
}
290+
291+
if (desc_len != length) {
292+
USB_LOG_ERR("Config descriptor length mismatch, expect %d, actual %d\r\n", length, desc_len);
293+
return -USB_ERR_INVAL;
294+
}
284295
}
285296
return 0;
286297
}
@@ -447,7 +458,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
447458
/* Read the full size of the configuration data */
448459
uint16_t wTotalLength = ((struct usb_configuration_descriptor *)ep0_request_buffer[hport->bus->busid])->wTotalLength;
449460

450-
if (wTotalLength > CONFIG_USBHOST_REQUEST_BUFFER_LEN) {
461+
if (wTotalLength >= CONFIG_USBHOST_REQUEST_BUFFER_LEN) {
451462
ret = -USB_ERR_NOMEM;
452463
USB_LOG_ERR("wTotalLength %d is overflow, default is %d\r\n", wTotalLength, (unsigned int)CONFIG_USBHOST_REQUEST_BUFFER_LEN);
453464
goto errout;
@@ -465,6 +476,7 @@ int usbh_enumerate(struct usbh_hubport *hport)
465476
goto errout;
466477
}
467478

479+
ep0_request_buffer[hport->bus->busid][wTotalLength] = '\0';
468480
ret = parse_config_descriptor(hport, (struct usb_configuration_descriptor *)ep0_request_buffer[hport->bus->busid], wTotalLength);
469481
if (ret < 0) {
470482
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
803815
struct usbh_hubport *hport;
804816
struct usbh_hub *hub_next;
805817

806-
if ((hub_index > hub->index) || (hub_port > hub->nports)) {
807-
return NULL;
808-
}
818+
USB_ASSERT((hub_index > 0) && (hub_port > 0) && (hub_index <= hub->index) && (hub_port <= hub->nports));
809819

810820
if (hub->index == hub_index) {
811821
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
869879
struct usbh_hubport *hport;
870880
size_t flags;
871881

882+
USB_ASSERT_MSG(busid < CONFIG_USBHOST_MAX_BUS, "bus overflow\r\n");
883+
872884
flags = usb_osal_enter_critical_section();
873885

874886
bus = &g_usbhost_bus[busid];

0 commit comments

Comments
 (0)