Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple buffer overflow vulnerabilities in TeenyUSB #18

Open
hac425xxx opened this issue Jun 8, 2021 · 1 comment
Open

multiple buffer overflow vulnerabilities in TeenyUSB #18

hac425xxx opened this issue Jun 8, 2021 · 1 comment

Comments

@hac425xxx
Copy link

buffer overflow and out of bound in tusb_rndis_device_request

static int tusb_rndis_device_request(tusb_rndis_device_t* cdc, tusb_setup_packet* setup_req)
{
    tusb_device_t* dev = cdc->dev;

	.......................
	.......................
	
    }else if(setup_req->bRequest == CDC_SEND_ENCAPSULATED_COMMAND){
        dev->ep0_rx_done = rndis_dataout_request;
        
        // overflow !!!
        tusb_set_recv_buffer(dev, 0, cdc->encapsulated_buffer, setup_req->wLength);
        tusb_set_rx_valid(dev, 0);
        return 1;
    }

if setup_req->wLength > the size of cdc->encapsulated_buffer, it could overflow.

struct _tusb_rndis_device
{
    tusb_device_t* dev;
	......................
	......................
    uint8_t    encapsulated_buffer[256];

if setup_req->wLength > 256 , it could overflow.

out of bound

    if(setup_req->bRequest == CDC_GET_ENCAPSULATED_RESPONSE){
        tusb_fix_control_xfer_corrupt_issue(dev);
        
        // vuln!!!
        tusb_control_send(dev, cdc->encapsulated_buffer, ((rndis_generic_msg_t *)cdc->encapsulated_buffer)->MessageLength );
        return 1;

cdc->encapsulated_buffer is receive from other usb device, which is recv in CDC_SEND_ENCAPSULATED_COMMAND case.

if MessageLength is too large, it could out of bound read.

out of bound read in rndis_handle_set_msg

cdc->encapsulated_buffer is receive from other usb device

static void rndis_handle_set_msg(tusb_rndis_device_t* cdc)
{
    rndis_set_cmplt_t *c;
    rndis_set_msg_t *m;
    rndis_Oid_t oid;

    c = (rndis_set_cmplt_t *)cdc->encapsulated_buffer;
    m = (rndis_set_msg_t *)cdc->encapsulated_buffer;
	
	...............
	...............

    oid = m->Oid;
    c->MessageType = REMOTE_NDIS_SET_CMPLT;
    c->MessageLength = sizeof(rndis_set_cmplt_t);
    c->Status = RNDIS_STATUS_SUCCESS;

    switch (oid)
    {
        case OID_GEN_RNDIS_CONFIG_PARAMETER:
            {
                rndis_config_parameter_t *p;
                char *ptr = (char *)m;
                ptr += sizeof(rndis_generic_msg_t);
                ptr += m->InformationBufferOffset;
                p = (rndis_config_parameter_t *)ptr;
                if(cdc->config_param){
                
                	// vuln !!!
                    cdc->config_param(cdc, ptr+p->ParameterNameOffset, ptr+p->ParameterValueOffset, p->ParameterNameLength, p->ParameterValueLength);
                }
            }
            break;

when m->InformationBufferOffset is large, it could out of bound read!

buffer overflow in tusb_cdc_device_request

static int tusb_cdc_device_request(tusb_cdc_device_t* cdc, tusb_setup_packet* setup_req)
{
  if( (setup_req->bmRequestType & USB_REQ_TYPE_MASK) == USB_REQ_TYPE_CLASS){
    if(setup_req->wLength > 0){ 
      	
      	// overflow !!!
        tusb_set_recv_buffer(dev, 0, dev_config->cmd_buffer, setup_req->wLength);

if setup_req->wLength > the size of dev_config->cmd_buffer, it could overflow.

buffer overflow in tusb_hid_device_request

static int tusb_hid_device_request(tusb_hid_device_t* hid, tusb_setup_packet* setup_req)
{
	  ......................
	  ......................
      case HID_REQ_SET_REPORT:
      	
      	// overflow !!!
        tusb_set_recv_buffer(dev, 0, dev_config->cmd_buffer, setup_req->wLength);
        tusb_set_rx_valid(dev, 0);

if setup_req->wLength > the size of dev_config->cmd_buffer, it could overflow.

out of bound read in msc_scsi_write_10

static int msc_scsi_write_10(tusb_msc_device_t* msc)
{
  tusb_msc_cbw_t * cbw = &msc->state.cbw;
  tusb_msc_csw_t       * csw = &msc->state.csw;
  scsi_read_10_cmd_t* cmd = (scsi_read_10_cmd_t*)cbw->command;
 	......
 	......
    uint32_t block_addr = GET_BE32(cmd->logical_block_addr);
    uint32_t block_size = cbw->total_bytes/ GET_BE16(cmd->transfer_length);
    int length = msc->state.data_out_length;
    uint16_t block_count = length/block_size;
    
    if(msc->block_write){
      // TODO: support data buffer length less than the block size
      int xferred_length = cbw->total_bytes - csw->data_residue;
      
      // vuln !!! 
      length = msc->block_write(msc, cbw->lun, msc->state.data_buffer, block_addr + xferred_length/block_size, block_count);

cbw and cmd is receive from other usb device, if block_count too large , it could lead oob read.

@xtoolbox
Copy link
Owner

  1. In MSC, we need check the length before invoke block_write, int length = min(msc->state.data_out_length, sizeof(msc->state.data_buffer));
  2. In HID and CDC, may need check the setup data length before next process, when buffer is small than request the stack may throw an error and return a stall state.
    The best way may dynamic allocate memory for the setup data buffer, it need implement malloc and free. But I want keep all memories static in device mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants