STMicroelectronics / STM32CubeH7

STM32Cube MCU Full Package for the STM32H7 series - (HAL + LL Drivers, CMSIS Core, CMSIS Device, MW libraries plus a set of Projects running on all boards provided by ST (Nucleo, Evaluation and Discovery Kits))
https://www.st.com/en/embedded-software/stm32cubeh7.html
Other
479 stars 298 forks source link

Potential buffer overflow in WiFi SPI driver #252

Closed shijiameng closed 11 months ago

shijiameng commented 1 year ago

Bug Description

static int8_t SPI_WIFI_ResetModule(void)
{
    uint32_t tickstart = HAL_GetTick();
    uint8_t Prompt[6];
    uint8_t count = 0;
    HAL_StatusTypeDef  Status;

    WIFI_RESET_MODULE();
    WIFI_ENABLE_NSS();
    SPI_WIFI_DelayUs(10);

    while (WIFI_IS_CMDDATA_READY())
    {
        Status = HAL_SPI_Receive(&hspi, &Prompt[count], 1, 0xFFFF);  // !!! Buggy line: bound check is missing !!!
        count += 2;
        if (((HAL_GetTick() - tickstart) > 0xFFFF) || (Status != HAL_OK))
        {
            WIFI_DISABLE_NSS();
            return -1;
        }
    }

    WIFI_DISABLE_NSS();
    if ((Prompt[0] != 0x15) || (Prompt[1] != 0x15) || (Prompt[2] != '\r') ||
            (Prompt[3] != '\n') || (Prompt[4] != '>') || (Prompt[5] != ' '))
    {
        return -1;
    }
    return 0;
}

Patch Suggestion

Add bound check before invoking HAL_SPI_Receive().

static int8_t SPI_WIFI_ResetModule(void)
{
    uint32_t tickstart = HAL_GetTick();
    uint8_t Prompt[6];
    uint8_t count = 0;
    HAL_StatusTypeDef  Status;

    WIFI_RESET_MODULE();
    WIFI_ENABLE_NSS();
    SPI_WIFI_DelayUs(10);

    while (WIFI_IS_CMDDATA_READY())
    {
        // Patch: bound check before receiving data from SPI
        if (count == 4) {
            WIFI_DISABLE_NSS();
            return -1;
        }

        Status = HAL_SPI_Receive(&hspi, &Prompt[count], 1, 0xFFFF);
        count += 2;
        ...
    }
    ...
    return 0;
}
ALABSTM commented 11 months ago

ST Internal Reference: 158546

ALABSTM commented 11 months ago

Hi @shijiameng,

Thank you for this report too.

With regards,

ALABSTM commented 11 months ago

Hi @shijiameng,

The issue you reported has been fixed internally and should be available in the frame of a future release, but not in this repository, rather on st.com.

Indeed, as you can read on the README.md file, starting from version 1.11.1, some projects have been removed from this repository, among which the one you are mentioning in your comment.

More details about this move can be found here.

https://github.com/STMicroelectronics/STM32CubeH7/blob/91d4c89228e766a968f8daebc78eb7143d95184e/README.md?plain=1#L16-L23

As there is no fix to publish on this repository consequently to this fix, please allow me to close this issue. Thank you for your comprehension.

With regards,