WindhoverLabs / airliner

7 stars 3 forks source link

Undefined behavior - hk unit test #353

Open mariaKt opened 7 months ago

mariaKt commented 7 months ago

The hk unit test contains an undefined behavior where out of bounds array access occurs. See paragraph 6.5.6/8 of the C18 standard for more details.

The undefined behavior occurs when the pointer variable OuterCpyEntry is dereferenced: https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/src/hk_utils.c#L195

The pointer variable OuterCpyEntry is initialized to point to elements of array StartOfCopyTable inside a loop as follows: https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/src/hk_utils.c#L188-L190

The value that the array pointer StartOfCopyTable has when the undefined behavior occurs is the value of the global pointer variable HK_AppData.CopyTablePtr, propagated to it through the call chain. The HK_AppData.CopyTablePtr global pointer variable is initialized in line 92 of the HK_AppMain_Test_Nominal test, one of the tests performed by the hk unit test. https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/unit_test/hk_app_test.c#L87-L92

Note that HK_AppData.CopyTablePtr is initialized here as an one element array. Given that, and the fact that the array is accessed in a loop that assumes more than one elements, we have an out of bounds array access.

Note that there is code between the original initialization of the global pointer variable HK_AppData.CopyTablePtr and the out of bounds access that can load an array into memory and overwrite the prior value of HK_AppData.CopyTablePtr to point to it. https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/src/hk_app.c#L308-L310 https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/src/hk_app.c#L337-L338

However, this code is skipped because the hooks CFE_TBL_Load and CFE_TBL_GetAddress (that would load the array and change the value of HK_AppData.CopyTablePtr respectively) are configured at the initialization of the test to only return a specified return code. https://github.com/WindhoverLabs/airliner/blob/2ab9c8717e0a42d40b0d014a22dbcc1ed7ec0eb1/apps/hk/fsw/unit_test/hk_app_test.c#L107-L111

A similar issue occurs for other tests performed by the hk unit test.