USBH_ParseEPDesc: ep_descriptor->wMaxPacketSize derived from malicious endpoint descriptor

[szymonh]

Summary

USB host implementation is vulnerable to buffer overflows due to implementation of USBH_ParseEPDesc when USBH_MAX_EP_PACKET_SIZE is equal to USBH_MAX_DATA_BUFFER.

Description

The function responsible for endpoint descriptor parsing USBH_ParseEPDesc as defined in usbh_ctlreq.c does not limit the value of ep_descriptor->wMaxPacketSize when USBH_MAX_EP_PACKET_SIZE is equal to USBH_MAX_DATA_BUFFER. By default USBH_MAX_EP_PACKET_SIZE is set to 0x400 (as in usbh_def.h) and USBH_MAX_DATA_BUFFER also equals to 0x400 (again as defined in usbh_def.h). In contrast usbh_conf_template.h defines USBH_MAX_DATA_BUFFER to be 0x200U.

#define USBH_MAX_EP_PACKET_SIZE 0x400U

#ifndef USBH_MAX_DATA_BUFFER
#define USBH_MAX_DATA_BUFFER 0x400U
#endif

With USBH_MAX_EP_PACKET_SIZE == USBH_MAX_DATA_BUFFER ep_descriptor->wMaxPacketSize is directly derived from the endpoint descriptor.

`
static USBH_StatusTypeDef USBH_ParseEPDesc(USBH_HandleTypeDef *phost, USBH_EpDescTypeDef *ep_descriptor,
uint8_t *buf)
{
USBH_StatusTypeDef status = USBH_OK;
ep_descriptor->bLength = *(uint8_t *)(buf + 0);
ep_descriptor->bDescriptorType = *(uint8_t *)(buf + 1);
ep_descriptor->bEndpointAddress = *(uint8_t *)(buf + 2);
ep_descriptor->bmAttributes = *(uint8_t *)(buf + 3);
ep_descriptor->wMaxPacketSize = LE16(buf + 4);
ep_descriptor->bInterval = *(uint8_t *)(buf + 6);

/* Make sure that wMaxPacketSize is different from 0 /
if (ep_descriptor->wMaxPacketSize == 0x00U)
{
status = USBH_NOT_SUPPORTED;
}
else if (USBH_MAX_EP_PACKET_SIZE < (uint16_t)USBH_MAX_DATA_BUFFER)
{
/ Make sure that maximum packet size (bits 0..10) does not exceed the max endpoint packet size */
ep_descriptor->wMaxPacketSize &= ~0x7FFU;
ep_descriptor->wMaxPacketSize |= MIN((uint16_t)(LE16(buf + 4) & 0x7FFU), (uint16_t)USBH_MAX_EP_PACKET_SIZE);

}
else if ((uint16_t)USBH_MAX_DATA_BUFFER < USBH_MAX_EP_PACKET_SIZE)
{
/* Make sure that maximum packet size (bits 0..10) does not exceed the total buffer length /
ep_descriptor->wMaxPacketSize &= ~0x7FFU;
ep_descriptor->wMaxPacketSize |= MIN((uint16_t)(LE16(buf + 4) & 0x7FFU), (uint16_t)USBH_MAX_DATA_BUFFER);
}
else
{
/ ... */
}
`

Consequently ep_descriptor->wMaxPacketSize value will be set as follows:

ep_descriptor->wMaxPacketSize = LE16(buf + 4);

In case of a malicious device the wMaxPacketSize value provided in the endpoint descriptor may have a unexpected large value like 0xffff. This case will result in buffer overflows in multiple locations in the USB Host stack like:

• CDC_ProcessReception : 738
• USBH_AUDIO_Control : 1627, 1653
• USBH_HID_Process : 444
• USBH_MSC_BOT_Process : 261, 289
• USBH_PTP_Process : 323, 359

Impact

Devices implementing usb host functionality based on stm32_mw_usb_host middleware are vulnerable to multiple buffer overflows due to endpoint descriptor parsing implemented in USBH_ParseEPDesc in case USBH_MAX_EP_PACKET_SIZE is equal to USBH_MAX_DATA_BUFFER. Depending on particular code location affected by unexpectedly large value of ep_descriptor->wMaxPacketSize this may result in write past buffer boundaries, bypass of security features or in the worst case scenario execution of arbitrary code.

Expected resolution

Assure that ep_descriptor->wMaxPacketSize is not permitted to be greater than USBH_MAX_EP_PACKET_SIZE when USBH_MAX_EP_PACKET_SIZE equals to USBH_MAX_DATA_BUFFER.

[ALABSTM]

Hi @szymonh,

Thank you very much for this exhaustive description and the proposal you made. It will be forwarded to our development teams. We will get back to you as soon as we have their feedback.

With regards,

[ALABSTM]

Fix proposal reworded:

The code bloc below is executed in case USBH_MAX_EP_PACKET_SIZE equals USBH_MAX_DATA_BUFFER. In this case, it must be ensured that ep_descriptor->wMaxPacketSize is not allowed to be greater than USBH_MAX_EP_PACKET_SIZE.

stm32_mw_usb_host/Core/Src/usbh_ctlreq.c

Lines 567 to 570 in 70f090d

	else
	{
	/* ... */
	}

[ALABSTM]

ST Internal Reference: 118017

[szymonh]

Hello @ALABSTM ,

Since this issue may result in a buffer overflow affecting application security are you planning to publish a corresponding CVE?

Best regards,
Szymon

[ALABSTM]

Hi @szymonh,

The issue you pointed out has been fixed in the frame of version 3.4.1 of this library. Please allow me to close this thread. Thank you again for having reported.

With regards,