stm32_mw_usb_device icon indicating copy to clipboard operation
stm32_mw_usb_device copied to clipboard

USBD_AUDIO_DataOut buffer overflow vulnerability

Open szymonh opened this issue 2 years ago • 3 comments

Summary

The implementation of USB Audio device class (USBD_AUDIO_DataOut handler) is vulnerable to a buffer overflow when a unexpected amount of data is read from the host.

Description

The data out stage handler (USBD_AUDIO_DataOut) for usb audio device class does not include proper enforcement of buffer boundaries. Manipulation of the amount of data read from the host (PacketSize variable) allows an attacker to prevent buffer pointer rollback and introduce a buffer overflow.

With initially haudio->wr_ptr set to AUDIO_TOTAL_BUF_SIZE - AUDIO_OUT_PACKET one may send a packet of length AUDIO_OUT_PACKET-1 which will increment the haudio->wr_ptr to AUDIO_TOTAL_BUF_SIZE-1. Next packet of size greater than one will overflow the haudio->buffer buffer bypassing pointer rollback (since haudio->wr_ptr != AUDIO_TOTAL_BUF_SIZE). Sending further packets will increase the overflow to arbitrary size, as required by an attacker.

  if (epnum == AUDIOOutEpAdd)
  {
    /* Get received data packet length */
    PacketSize = (uint16_t)USBD_LL_GetRxDataSize(pdev, epnum);

    /* Packet received Callback */
    ((USBD_AUDIO_ItfTypeDef *)pdev->pUserData[pdev->classId])->PeriodicTC(&haudio->buffer[haudio->wr_ptr],
                                                                          PacketSize, AUDIO_OUT_TC);

    /* Increment the Buffer pointer or roll it back when all buffers are full */
    haudio->wr_ptr += PacketSize;

    if (haudio->wr_ptr == AUDIO_TOTAL_BUF_SIZE)
    {
      /* All buffers are full: roll back */
      haudio->wr_ptr = 0U;

      if (haudio->offset == AUDIO_OFFSET_UNKNOWN)
      {
        ((USBD_AUDIO_ItfTypeDef *)pdev->pUserData[pdev->classId])->AudioCmd(&haudio->buffer[0],
                                                                            AUDIO_TOTAL_BUF_SIZE / 2U,
                                                                            AUDIO_CMD_START);
        haudio->offset = AUDIO_OFFSET_NONE;
      }
    }

    if (haudio->rd_enable == 0U)
    {
      if (haudio->wr_ptr == (AUDIO_TOTAL_BUF_SIZE / 2U))
      {
        haudio->rd_enable = 1U;
      }
    }

    /* Prepare Out endpoint to receive next audio packet */
    (void)USBD_LL_PrepareReceive(pdev, AUDIOOutEpAdd,
                                 &haudio->buffer[haudio->wr_ptr],
                                 AUDIO_OUT_PACKET);

Impact

This issue allows an attacker to introduce a buffer overflow to a usb audio class device by exploiting the implementation of USBD_AUDIO_DataOut handler. Depending on particular implementation this issue may result in write past buffer boundaries, bypass of security features or in the worst case scenario execution of arbitrary code.

Expected resolution

The implementation may be fixed by altering pointer roll back condition from equals to greater or equal.

    /* Increment the Buffer pointer or roll it back when all buffers are full */
    haudio->wr_ptr += PacketSize;

    if (haudio->wr_ptr >= AUDIO_TOTAL_BUF_SIZE)
    {
      /* All buffers are full: roll back */
      haudio->wr_ptr = 0U;

szymonh avatar Nov 30 '21 21:11 szymonh

ST Internal Reference: 119956

ALABSTM avatar Dec 22 '21 18:12 ALABSTM

Hi @szymonh,

Thank you for this other report. It has been reported to our development teams. The check at the line below shall be updated.

https://github.com/STMicroelectronics/stm32_mw_usb_device/blob/69fa8a86adf2877ff1e5e40af527ff470194c54f/Class/AUDIO/Src/usbd_audio.c#L786

With regards,

ALABSTM avatar Dec 22 '21 18:12 ALABSTM

Hello @ALABSTM ,

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

Best regards, Szymon

szymonh avatar Dec 27 '21 19:12 szymonh