Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 30 additions & 8 deletions src/class/cdc/cdc_host.c
Original file line number Diff line number Diff line change
Expand Up @@ -714,9 +714,21 @@ bool cdch_xfer_cb(uint8_t daddr, uint8_t ep_addr, xfer_result_t event, uint32_t
//--------------------------------------------------------------------+
// Enumeration
//--------------------------------------------------------------------+

// Descriptor-walk hardening gated by CFG_TUH_VALIDATION_LEVEL: at NONE the guards collapse to a pass so
// trusted-device setups pay no code size; at BASIC (default) the walk stays inside the enumeration buffer.
#if CFG_TUH_VALIDATION_LEVEL >= TUSB_VALIDATION_BASIC
#define TU_DESC_VALIDATE(_cond) (_cond)
#else
#define TU_DESC_VALIDATE(_cond) (true)
#endif

static bool open_ep_stream_pair(cdch_interface_t *p_cdc, tusb_desc_endpoint_t const *desc_ep) {
for (size_t i = 0; i < 2; i++) {
TU_ASSERT(TUSB_DESC_ENDPOINT == desc_ep->bDescriptorType && TUSB_XFER_BULK == desc_ep->bmAttributes.xfer, 0);
// pin bLength so tu_desc_next() below cannot walk the second endpoint past a caller-checked bound
TU_ASSERT(TU_DESC_VALIDATE(sizeof(tusb_desc_endpoint_t) == desc_ep->bLength) &&
TUSB_DESC_ENDPOINT == desc_ep->bDescriptorType && TUSB_XFER_BULK == desc_ep->bmAttributes.xfer,
0);
TU_ASSERT(tuh_edpt_open(p_cdc->daddr, desc_ep));
const uint8_t ep_dir = tu_edpt_dir(desc_ep->bEndpointAddress);
tu_edpt_stream_t *stream = (ep_dir == TUSB_DIR_IN) ? &p_cdc->stream.rx : &p_cdc->stream.tx;
Expand Down Expand Up @@ -1020,8 +1032,12 @@ static uint16_t acm_open(uint8_t daddr, const tusb_desc_interface_t *itf_desc, u
p_desc = tu_desc_next(p_desc);

// Communication Functional Descriptors
while ((p_desc < desc_end) && (TUSB_DESC_CS_INTERFACE == tu_desc_type(p_desc))) {
if (CDC_FUNC_DESC_ABSTRACT_CONTROL_MANAGEMENT == cdc_functional_desc_typeof(p_desc)) {
// need the 3-byte header (bLength/bDescriptorType/bDescriptorSubType) in bounds before reading it, and a
// bLength >= 3 both keeps those reads valid and stops a zero-length descriptor from spinning the walk
while ((p_desc < desc_end) && TU_DESC_VALIDATE(p_desc + 3 <= desc_end) &&
TUSB_DESC_CS_INTERFACE == tu_desc_type(p_desc) && TU_DESC_VALIDATE(tu_desc_len(p_desc) >= 3)) {
if (CDC_FUNC_DESC_ABSTRACT_CONTROL_MANAGEMENT == cdc_functional_desc_typeof(p_desc) &&
TU_DESC_VALIDATE(p_desc + sizeof(cdc_desc_func_acm_t) <= desc_end)) {
// save ACM bmCapabilities
p_cdc->acm.capability = ((cdc_desc_func_acm_t const *) p_desc)->bmCapabilities;
}
Expand All @@ -1031,23 +1047,29 @@ static uint16_t acm_open(uint8_t daddr, const tusb_desc_interface_t *itf_desc, u

// Open notification endpoint of control interface if any
if (itf_desc->bNumEndpoints == 1) {
// whole endpoint descriptor must fit: tuh_edpt_open reads the full struct regardless of bLength
TU_ASSERT(TU_DESC_VALIDATE(p_desc + sizeof(tusb_desc_endpoint_t) <= desc_end), 0);
TU_ASSERT(TUSB_DESC_ENDPOINT == tu_desc_type(p_desc), 0);
Comment on lines 1048 to 1052

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, the functional descriptor walk was still trusting the loop guard, so a short or zero-length CS_INTERFACE descriptor near desc_end could read the subtype past the end (or spin on bLength==0). i've tightened the loop to require the 3-byte header in bounds plus bLength >= 3 before it reads the type/subtype, and the bmCapabilities read now only happens when the whole cdc_desc_func_acm_t fits inside desc_end.

const tusb_desc_endpoint_t *desc_ep = (const tusb_desc_endpoint_t *)p_desc;
TU_ASSERT(tuh_edpt_open(daddr, desc_ep), 0);
Comment on lines +1050 to 1054

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, tu_desc_next there was still advancing by the device bLength, so an inflated value could push p_desc past desc_end before the next check even ran. since we've already asserted the whole 7-byte endpoint fits, i now step forward by sizeof(tusb_desc_endpoint_t) instead, which keeps the pointer inside the window we validated.

p_cdc->ep_notif = desc_ep->bEndpointAddress;

p_desc = tu_desc_next(p_desc);
// advance by the fixed struct size, not device-supplied bLength, so p_desc stays inside the checked window
p_desc += sizeof(tusb_desc_endpoint_t);
}

//------------- Data Interface (if any) -------------//
if (TUSB_DESC_INTERFACE == tu_desc_type(p_desc)) {
if (TU_DESC_VALIDATE(p_desc + sizeof(tusb_desc_interface_t) <= desc_end) &&
TUSB_DESC_INTERFACE == tu_desc_type(p_desc)) {
const tusb_desc_interface_t *data_itf = (const tusb_desc_interface_t *)p_desc;
if (data_itf->bInterfaceClass == TUSB_CLASS_CDC_DATA) {
p_desc = tu_desc_next(p_desc); // next to endpoint descriptor
p_desc += sizeof(tusb_desc_interface_t); // fixed struct size to endpoint descriptor, not device bLength

// data endpoints expected to be in pairs
// open_ep_stream_pair consumes exactly two endpoints; require that count and that both fit before reading them
TU_ASSERT(TU_DESC_VALIDATE(data_itf->bNumEndpoints == 2), 0);
TU_ASSERT(TU_DESC_VALIDATE(p_desc + 2 * sizeof(tusb_desc_endpoint_t) <= desc_end), 0);
TU_ASSERT(open_ep_stream_pair(p_cdc, (const tusb_desc_endpoint_t *)p_desc), 0);
p_desc += data_itf->bNumEndpoints * sizeof(tusb_desc_endpoint_t);
p_desc += 2 * sizeof(tusb_desc_endpoint_t);
}
}

Expand Down
13 changes: 13 additions & 0 deletions src/tusb_option.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,15 @@
#define OPT_MODE_HIGH_SPEED 0x0400u ///< High Speed
#define OPT_MODE_SPEED_MASK 0xff00u

//--------------------------------------------------------------------+
// Descriptor Validation Level
// How much the stack hardens itself against mal-configured or hostile devices, traded against code size.
// Higher levels add more checks; set CFG_TUD_VALIDATION_LEVEL / CFG_TUH_VALIDATION_LEVEL to pick one.
//--------------------------------------------------------------------+
#define TUSB_VALIDATION_NONE 0 ///< trusted devices only, minimal code size
#define TUSB_VALIDATION_BASIC 1 ///< default: mal-configured devices, no OOB reads / zero-length loops
#define TUSB_VALIDATION_STRICT 2 ///< reject malformed/hostile descriptors, stricter class validation

//--------------------------------------------------------------------+
// Include tusb_config.h
//--------------------------------------------------------------------+
Expand Down Expand Up @@ -694,6 +703,10 @@
#ifndef CFG_TUH_ENUMERATION_BUFSIZE
#define CFG_TUH_ENUMERATION_BUFSIZE 256
#endif

#ifndef CFG_TUH_VALIDATION_LEVEL
#define CFG_TUH_VALIDATION_LEVEL TUSB_VALIDATION_BASIC
#endif
#endif // CFG_TUH_ENABLED

// Attribute to place data in accessible RAM for host controller (default: CFG_TUSB_MEM_SECTION)
Expand Down
Loading