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

USBD_AUDIO_DataOut buffer overflow vulnerability #5

Closed
szymonh opened this issue Nov 30, 2021 · 4 comments
Closed

USBD_AUDIO_DataOut buffer overflow vulnerability #5

szymonh opened this issue Nov 30, 2021 · 4 comments
Assignees
Labels
enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system mw Middleware-related issue or pull-request. usb USB-related (host or device) issue or pull-request
Milestone

Comments

@szymonh
Copy link

szymonh commented Nov 30, 2021

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;
@ALABSTM ALABSTM added bug Something isn't working internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system usb USB-related (host or device) issue or pull-request enhancement New feature or request and removed bug Something isn't working labels Dec 22, 2021
@ALABSTM
Copy link
Collaborator

ALABSTM commented Dec 22, 2021

ST Internal Reference: 119956

@ALABSTM
Copy link
Collaborator

ALABSTM commented Dec 22, 2021

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.

if (haudio->wr_ptr == AUDIO_TOTAL_BUF_SIZE)

With regards,

@szymonh
Copy link
Author

szymonh commented Dec 27, 2021

Hello @ALABSTM ,

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

Best regards,
Szymon

@HBOSTM HBOSTM added the mw Middleware-related issue or pull-request. label Feb 21, 2023
@ALABSTM ALABSTM added this to the v2.11.0 milestone Apr 10, 2023
@ALABSTM
Copy link
Collaborator

ALABSTM commented Apr 10, 2023

Hi @szymonh,

I hope you are fine. The issue you reported has been fixed in the frame of version 2.11.0 of this library. Regarding the corresponding CVE, please refer to the SECURITY.md file to get the contact of our PSIRT team.

With regards,

@ALABSTM ALABSTM closed this as completed Apr 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request internal bug tracker Issue confirmed and reported into a ticket in the internal bug tracking system mw Middleware-related issue or pull-request. usb USB-related (host or device) issue or pull-request
Projects
None yet
Development

No branches or pull requests

3 participants