Dangerous VLA allocation In `stsafea_frame_receive()`

[parmi93]

A Variable Length Array (VLA) is allocated based on the response received from the STSAFE chip via I2C. However, no sanity check is performed on the response before the allocation. If the response is corrupted, for instance, due to a bit-flip caused by noise on the I2C line, it could trigger a VLA allocation of up to 65 KiB. This would almost certainly lead to a stack overflow and an application crash.

STSELib/services/stsafea/stsafea_frame_transfer.c

Line 184 in 3b8d263

PLAT_UI8 filler[filler_size];

[nils-cercariolo-st]

Thanks ! That's indeed a great catch, we'll review it and provide a fix. We should perform a sanity check with some bounds, we could take the upper bound as the STSAFE-AXXX I2C max buffer size. Feel free to share your thoughts if you think you have a better solution !

[parmi93]

I am not very familiar with the architectural design of STSELib, so it is difficult for me to suggest an appropriate upper bound, especially since I was not able to clearly understand the purpose of filler_size. That said, some form of sanity check would still seem advisable.

In addition, I noticed that filler_size == 0 is considered a valid value, as noted in the comment above. It is worth mentioning that, in standard C, defining a VLA with zero elements results in undefined behavior, whereas in the GCC dialect, zero-length arrays are permitted. For this reason, it might be helpful if the STSELib documentation could clarify which language standard was used to develop the library (I assume "GCC dialect" was used), so that users can clearly understand which C compilers are supported, particularly since the library states that it works across "various host MCU/MPU ecosystems", and not all of these ecosystems rely on GCC as their compiler.

[myorbit]

Thanks for the discussion so far. I would like to add some context from my integration scenario.

Although STSELib is primarily validated on STM32 MCUs, I'm integrating it on another MCU family/toolchain. In this environment (C11 dialect), VLAs are not allocated on the stack but are emulated by the run-time support (RTS) library using malloc/free with a relatively large fixed internal block size (~32 KiB). While this is functionally correct, it introduces avoidable heap usage and non-deterministic behavior on memory-constrained embedded targets.

You (@nils-cercariolo-st) mentioned the idea of defining an upper bound (e.g. a maximum I2C buffer size). Knowing a strict upper bound would allow memory to be managed explicitly rather than implicitly.

Based on that the requirement would be:

• All temporary buffers used by the library should have a well-defined upper bound known at compile time, so that memory usage is explicit and predictable.

How this upper bound is backed into memory could then be an implementation choice, for example:

• a fixed-size buffer (internal or caller provided), or
• application-managed backing storage (e.g. a fixed-block memory pool)

This would also be beneficial for our safety-related application because it improves determinism, makes worst-case usage analyzable and avoids hidden dynamic allocations.