10 buffer overflow in rt-thread usb stack

hac425xxx commented 3 years ago

code link

https://github.com/RT-Thread/rt-thread/blob/master/components/drivers/usb/usbdevice/class/rndis.c#L900

static rt_err_t _ep_out_handler(ufunction_t func, rt_size_t size)
{
    cdc_eps_t eps;
    char* data = RT_NULL;

    eps = (cdc_eps_t)&((rt_rndis_eth_t)func->user_data)->eps;
    data = (char*)eps->ep_out->buffer;

    if(((rt_rndis_eth_t)func->user_data)->rx_frist == RT_TRUE)
    {
        rndis_packet_msg_t msg = (rndis_packet_msg_t)data;

        ((rt_rndis_eth_t)func->user_data)->rx_length = msg->DataLength;
        ((rt_rndis_eth_t)func->user_data)->rx_offset = 0;

        if (size >= 44)
        {
            data += sizeof(struct rndis_packet_msg);
            size -= sizeof(struct rndis_packet_msg);
            ((rt_rndis_eth_t)func->user_data)->rx_frist = RT_FALSE;

            // bug !!!
            memcpy(&((rt_rndis_eth_t)func->user_data)->rx_buffer[((rt_rndis_eth_t)func->user_data)->rx_offset], data, size);
            ((rt_rndis_eth_t)func->user_data)->rx_offset += size;
        }

If size is greater than the remaining size of rx_buffer, it could overflow.

hac425xxx commented 3 years ago

bug 2

path

components/drivers/usb/usbdevice/class/ecm.c

code

/**
 * This function will handle RNDIS bulk out endpoint request.
 *
 * @param device the usb device object.
 * @param size request size.
 *
 * @return RT_EOK.
 */
static rt_err_t _ep_out_handler(ufunction_t func, rt_size_t size)
{
    rt_ecm_eth_t ecm_device = (rt_ecm_eth_t)func->user_data;

    // vuln !!!
    rt_memcpy((void *)(ecm_device->rx_buffer + ecm_device->rx_offset),ecm_device->rx_pool,size);
    ecm_device->rx_offset += size;
    if(size < EP_MAXPACKET(ecm_device->eps.ep_out))
    {
        ecm_device->rx_size = ecm_device->rx_offset;
        ecm_device->rx_offset = 0;
        eth_device_ready(&ecm_device->parent);

    }else
    {
        ecm_device->eps.ep_out->request.buffer = ecm_device->eps.ep_out->buffer;
        ecm_device->eps.ep_out->request.size = EP_MAXPACKET(ecm_device->eps.ep_out);
        ecm_device->eps.ep_out->request.req_type = UIO_REQUEST_READ_BEST;
        rt_usbd_io_request(ecm_device->func->device, ecm_device->eps.ep_out, &ecm_device->eps.ep_out->request);
    }

    return RT_EOK;
}

If size is greater than the remaining size of ecm_device->rx_buffer , it could overflow.

hac425xxx commented 3 years ago

bug 3

path

components/drivers/usb/usbhost/core/usbhost_core.c

code

    /* get device descriptor head */
    ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, 8);

    ......................................
    ......................................

    /* get full device descriptor again */
    // bug !!!
    ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, dev_desc->bLength);

dev_desc is receive from other usb device, if dev_desc->bLength > the size of dev_desc, it could buffer overflow.

hac425xxx commented 3 years ago

bug 4 - out of bound read in rt_usbh_get_interface_descriptor

path

components/drivers/usb/usbhost/core/usbhost_core.c

code

rt_err_t rt_usbh_get_interface_descriptor(ucfg_desc_t cfg_desc, int num,
    uintf_desc_t* intf_desc)
{

    ptr = (rt_uint32_t)cfg_desc + cfg_desc->bLength;
    while(ptr < (rt_uint32_t)cfg_desc + cfg_desc->wTotalLength)
    {
        // bug
        desc = (udesc_t)ptr;
        if(desc->type == USB_DESC_TYPE_INTERFACE)

if ptr == cfg_desc + cfg_desc->wTotalLength - 1 , it could trigger out of bound read in desc->type .

hac425xxx commented 3 years ago

bug 5 - out of bound read in rt_usbh_get_endpoint_descriptor

path

components/drivers/usb/usbhost/core/usbhost_core.c

code

rt_err_t rt_usbh_get_endpoint_descriptor(uintf_desc_t intf_desc, int num,
    uep_desc_t* ep_desc)
{
    int count = 0, depth = 0;
    rt_uint32_t ptr;
    udesc_t desc;

    /* check parameter */
    RT_ASSERT(intf_desc != RT_NULL);
    RT_ASSERT(num < intf_desc->bNumEndpoints);
    *ep_desc = RT_NULL;

    ptr = (rt_uint32_t)intf_desc + intf_desc->bLength;
    while(count < intf_desc->bNumEndpoints)
    {
        if(depth++ > 0x20)
        {
            *ep_desc = RT_NULL;
            return -RT_EIO;
        }
        desc = (udesc_t)ptr;
        if(desc->type == USB_DESC_TYPE_ENDPOINT)
        {
            if(num == count)
            {
                *ep_desc = (uep_desc_t)desc;

                RT_DEBUG_LOG(RT_DEBUG_USB,
                             ("rt_usb_get_endpoint_descriptor: %d\n", num));
                return RT_EOK;
            }
            else count++;
        }
        ptr = (rt_uint32_t)desc + desc->bLength;
    }

    rt_kprintf("rt_usb_get_endpoint_descriptor %d failed\n", num);
    return -RT_EIO;
}

bug

ptr = (rt_uint32_t)desc + desc->bLength;

it don't verify ptr , it could lead out of bound read.

hac425xxx commented 3 years ago

bug 6 - out of bound read in _rndis_set_response

path

components\drivers\usb\usbdevice\class\rndis.c

code

static rt_err_t _rndis_set_response(ufunction_t func,rndis_set_msg_t msg)
{
    rndis_set_cmplt_t resp;
    struct rt_rndis_response * response;

    response = rt_malloc(sizeof(struct rt_rndis_response));
    resp = rt_malloc(sizeof(struct rndis_set_cmplt));

    resp->RequestId = msg->RequestId;
    resp->MessageType = REMOTE_NDIS_SET_CMPLT;
    resp->MessageLength = sizeof(struct rndis_set_cmplt);

    switch (msg->Oid)
    {
    case OID_GEN_CURRENT_PACKET_FILTER:

        // bug !!!
        oid_packet_filter = *((rt_uint32_t *)((rt_uint8_t *)&(msg->RequestId) + \
                                              msg->InformationBufferOffset));

msg is recv from other usb device, if msg->InformationBufferOffset is too large, it could lead to out-of-bound read.

hac425xxx commented 3 years ago

bug 7 - buffer overflow in winusb.c: _ep0_cmd_read

path

components\drivers\usb\usbdevice\class\winusb.c

code

static rt_err_t _ep0_cmd_read(ufunction_t func, ureq_t setup)
{
    winusb_device_t winusb_device = (winusb_device_t)func->user_data;
    cmd_func = func;

    // bug !!!
    rt_usbd_ep0_read(func->device,winusb_device->cmd_buff,setup->wLength,_ep0_cmd_handler);
    return RT_EOK;
}

setup->wLength is recv from other usb device, if setup->wLength is too large, it could lead to winusb_device->cmd_buff overflow.

hac425xxx commented 3 years ago

bug 8 - buffer overflow in umouse.c: mouse_task

path

components\drivers\usb\usbhost\class\umouse.c

code

void mouse_task(void* param)
{
    struct uhintf* intf = (struct uhintf*)param;
    while (1)
    {
        // bug !!!
        if (rt_usb_hcd_pipe_xfer(intf->device->hcd, ((struct uhid*)intf->user_data)->pipe_in,
            ((struct uhid*)intf->user_data)->buffer, ((struct uhid*)intf->user_data)->pipe_in->ep.wMaxPacketSize,
            USB_TIMEOUT_BASIC) == 0)
        {
            break;
        }

        rt_usbh_hid_mouse_callback(intf->user_data);
    }
}

ep.wMaxPacketSize is recv from other usb device, if ep.wMaxPacketSize > sizeof(buffer) , it could overflow.

struct uhid
{
    upipe_t pipe_in;
    rt_uint8_t buffer[8];
    uprotocal_t protocal;
};

the size of buffer is only 8 bytes.

hac425xxx commented 3 years ago

bug 9 - buffer overflow in rt_usbh_hub_enable

path

components\drivers\usb\usbhost\core\hub.c

code

static rt_err_t rt_usbh_hub_enable(void *arg)
{

    /* get hub descriptor head */
    ret = rt_usbh_hub_get_descriptor(device, (rt_uint8_t*)&hub->hub_desc, 8);
    if(ret != RT_EOK)
    {
        rt_kprintf("get hub descriptor failed\n");
        return -RT_ERROR;
    }

    /* get full hub descriptor */
    ret = rt_usbh_hub_get_descriptor(device, (rt_uint8_t*)&hub->hub_desc,
        hub->hub_desc.length);
    if(ret != RT_EOK)

hub->hub_desc.length is recv from other usb device, if hub->hub_desc.length > the size of hub->hub_desc , it could overflow.

hac425xxx commented 3 years ago

bug 10 - buffer overflow in rt_usbh_hub_irq

path

components\drivers\usb\usbhost\core\hub.c

code

static void rt_usbh_hub_irq(void* context)
{
    upipe_t pipe;
    uhub_t hub;

    pipe = (upipe_t)context;

    rt_usb_hcd_pipe_xfer(hub->self->hcd, pipe, hub->buffer, pipe->ep.wMaxPacketSize, timeout);
}

ep.wMaxPacketSize is recv from other usb device, if ep.wMaxPacketSize > sizeof(hub->buffer) , it could overflow.

thewon86 commented 3 years ago

bug 3 在gitee上的一个pr里已经修改，不过，修改前也没有风险，因为 dev_desc->bLength 刚好等于 dev_desc 结构体大小！虽然是这么说在这个pr里我把 dev_desc 和 cfg_desc 做了统一操作。可以看这个 PR

bug 4 你可能也注意到了，cfg_desc 结构体定义最后有个 data ，这个部分其实是变长的，不定长度！原操作就是让 ptr 指向这个 data 内存位置，所以原操作没错！

bug 5 的检查是通过count的

bug 9 hub_desc 同 dev_desc，但是这次因为没涉及到，上述 pr 没修改这里。

其它 bug ，这些地方都在硬件和 usb 协议规范内明确规定的，除非，硬件线路有干扰，所有数据都不正常了，或者接入了一个非法硬件。又或者是上层调用者考虑过了，下层就不需要重复判断。

hac425xxx commented 3 years ago

首先攻击模型是：一个恶意的USB设备插到了我们系统上，所以凡是从USB设备收上来的数据都需要检查

bug3 ：

程序的逻辑是，首先从usb设备收 8 字节作为 dev_desc

/* get device descriptor head */
ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, 8);

然后根据 dev_desc->bLength 从 usb 设备里收数据

    ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, dev_desc->bLength);

所以 dev_desc->bLength 是恶意usb设备控制的，如果usb设备返回一个非常大的值，在这里收数据的时候就会溢出

hac425xxx commented 3 years ago

bug 4

程序首先从 USB 总线上收 device->cfg_desc

    /* get full configuration descriptor */
    ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_CONFIGURATION,
        device->cfg_desc, cfg_desc.wTotalLength);

然后调用 rt_usbh_get_interface_descriptor 进行解析

rt_err_t rt_usbh_get_interface_descriptor(ucfg_desc_t cfg_desc, int num,
    uintf_desc_t* intf_desc)
{
    rt_uint32_t ptr, depth = 0;
    udesc_t desc;

    /* check parameter */
    RT_ASSERT(cfg_desc != RT_NULL);

    ptr = (rt_uint32_t)cfg_desc + cfg_desc->bLength;
    while(ptr < (rt_uint32_t)cfg_desc + cfg_desc->wTotalLength)

其中 cfg_desc 是从USB总线上收上来的数据，所以需要检查

hac425xxx commented 3 years ago

其他问题也是类似 https://www.cnblogs.com/hac425/p/14872442.html#rt-thread-usb-%E5%8D%8F%E8%AE%AE%E6%A0%88

hac425xxx commented 3 years ago

一些知名的USB攻击案例

https://paper.seebug.org/1065/ https://github.com/Ginurx/fusee_gelee_explained_in_chinese

thewon86 commented 3 years ago

一些知名的USB攻击案例

https://paper.seebug.org/1065/ https://github.com/Ginurx/fusee_gelee_explained_in_chinese

这样就很严重了

lymzzyh commented 3 years ago

首先攻击模型是：一个恶意的USB设备插到了我们系统上，所以凡是从USB设备收上来的数据都需要检查

bug3 ：

程序的逻辑是，首先从usb设备收 8 字节作为 dev_desc
/* get device descriptor head */
ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, 8);
然后根据 dev_desc->bLength 从 usb 设备里收数据
    ret = rt_usbh_get_descriptor(device, USB_DESC_TYPE_DEVICE, (void*)dev_desc, dev_desc->bLength);
所以 dev_desc->bLength 是恶意usb设备控制的，如果usb设备返回一个非常大的值，在这里收数据的时候就会溢出

这么说确实没毛病。修

lymzzyh commented 3 years ago

host 协议栈正在重新梳理中

lymzzyh commented 3 years ago

code link

https://github.com/RT-Thread/rt-thread/blob/master/components/drivers/usb/usbdevice/class/rndis.c#L900

static rt_err_t _ep_out_handler(ufunction_t func, rt_size_t size)
{
    cdc_eps_t eps;
    char* data = RT_NULL;

    eps = (cdc_eps_t)&((rt_rndis_eth_t)func->user_data)->eps;
    data = (char*)eps->ep_out->buffer;

    if(((rt_rndis_eth_t)func->user_data)->rx_frist == RT_TRUE)
    {
        rndis_packet_msg_t msg = (rndis_packet_msg_t)data;

        ((rt_rndis_eth_t)func->user_data)->rx_length = msg->DataLength;
        ((rt_rndis_eth_t)func->user_data)->rx_offset = 0;

        if (size >= 44)
        {
            data += sizeof(struct rndis_packet_msg);
            size -= sizeof(struct rndis_packet_msg);
            ((rt_rndis_eth_t)func->user_data)->rx_frist = RT_FALSE;

            // bug !!!
            memcpy(&((rt_rndis_eth_t)func->user_data)->rx_buffer[((rt_rndis_eth_t)func->user_data)->rx_offset], data, size);
            ((rt_rndis_eth_t)func->user_data)->rx_offset += size;
        }

If size is greater than the remaining size of rx_buffer, it could overflow.

这个地方我仔细看了看是不可能overflow的，因为这是收取的第一个数据包此时 offset为0， buffer大小为RNDIS头+MTU+14的大小。而size最大最大就是rt_rndis_eth_rx中请求的EP_MAXPACKET(device->eps.ep_out)大小即使是HighSpeed设备也才512 是不可能溢出的，不过在后续数据包中的size判定的确需要做一个限制，以防主机进行攻击

lymzzyh commented 3 years ago

bug 2

path

components/drivers/usb/usbdevice/class/ecm.c

code

/**
 * This function will handle RNDIS bulk out endpoint request.
 *
 * @param device the usb device object.
 * @param size request size.
 *
 * @return RT_EOK.
 */
static rt_err_t _ep_out_handler(ufunction_t func, rt_size_t size)
{
    rt_ecm_eth_t ecm_device = (rt_ecm_eth_t)func->user_data;

    // vuln !!!
    rt_memcpy((void *)(ecm_device->rx_buffer + ecm_device->rx_offset),ecm_device->rx_pool,size);
    ecm_device->rx_offset += size;
    if(size < EP_MAXPACKET(ecm_device->eps.ep_out))
    {
        ecm_device->rx_size = ecm_device->rx_offset;
        ecm_device->rx_offset = 0;
        eth_device_ready(&ecm_device->parent);

    }else
    {
        ecm_device->eps.ep_out->request.buffer = ecm_device->eps.ep_out->buffer;
        ecm_device->eps.ep_out->request.size = EP_MAXPACKET(ecm_device->eps.ep_out);
        ecm_device->eps.ep_out->request.req_type = UIO_REQUEST_READ_BEST;
        rt_usbd_io_request(ecm_device->func->device, ecm_device->eps.ep_out, &ecm_device->eps.ep_out->request);
    }

    return RT_EOK;
}

If size is greater than the remaining size of ecm_device->rx_buffer , it could overflow.

ECM整个接收逻辑有问题，也不知道我当时是怎么想的这部分我直接重写吧

lymzzyh commented 3 years ago

bug 7 - buffer overflow in winusb.c: _ep0_cmd_read

path
components\drivers\usb\usbdevice\class\winusb.c
code
static rt_err_t _ep0_cmd_read(ufunction_t func, ureq_t setup)
{
    winusb_device_t winusb_device = (winusb_device_t)func->user_data;
    cmd_func = func;

    // bug !!!
    rt_usbd_ep0_read(func->device,winusb_device->cmd_buff,setup->wLength,_ep0_cmd_handler);
    return RT_EOK;
}
setup->wLength is recv from other usb device, if setup->wLength is too large, it could lead to winusb_device->cmd_buff overflow.

winusb只是一个模板驱动由用户自己编写此部分不做处理

thewon86 commented 3 years ago

@lymzzyh 你是 usbhost 框架原创建者？有个问题请教一下，有办法提高 rt_usb_hcd_pipe_xfer 函数中 send_size = (remain_size > pipe->ep.wMaxPacketSize) ? pipe->ep.wMaxPacketSize : remain_size; 这里用到的 “pipe->ep.wMaxPacketSize” 值的大小吗？ 64字节分一次，stm32 那边又要求必须有个 1ms 延时，分包越小，延时次数越多，挺影响速度的，这个有办法解决吗？

lymzzyh commented 3 years ago

@lymzzyh 你是 usbhost 框架原创建者？有个问题请教一下，有办法提高 rt_usb_hcd_pipe_xfer 函数中 send_size = (remain_size > pipe->ep.wMaxPacketSize) ? pipe->ep.wMaxPacketSize : remain_size; 这里用到的 “pipe->ep.wMaxPacketSize” 值的大小吗？ 64字节分一次，stm32 那边又要求必须有个 1ms 延时，分包越小，延时次数越多，挺影响速度的，这个有办法解决吗？

全速模式下最大就只能64字节。

thewon86 commented 3 years ago

@lymzzyh 你是 usbhost 框架原创建者？有个问题请教一下，有办法提高 rt_usb_hcd_pipe_xfer 函数中 send_size = (remain_size > pipe->ep.wMaxPacketSize) ? pipe->ep.wMaxPacketSize : remain_size; 这里用到的 “pipe->ep.wMaxPacketSize” 值的大小吗？ 64字节分一次，stm32 那边又要求必须有个 1ms 延时，分包越小，延时次数越多，挺影响速度的，这个有办法解决吗？

全速模式下最大就只能64字节。

好吧，谢谢，理论速度最高只有 64 k了。

RT-Thread / rt-thread

10 buffer overflow in rt-thread usb stack #4776