cFE icon indicating copy to clipboard operation
cFE copied to clipboard

SCA Finding in CFE_ES_StartAppTask

Open ejtimmon opened this issue 2 years ago • 2 comments

Describe the bug Klockwork static analysis tool flagged the following finding:

File: /ccrs/flight-sw/fsw/cfe/modules/es/fsw/src/cfe_es_apps.c Line: 607 Function: CFE_ES_StartAppTask Finding: Result of function that may return NULL will be dereferenced

To Reproduce Run Klocwork tool

Reporter Info Beth Geist/NASA GSFC

ejtimmon avatar Oct 23 '23 14:10 ejtimmon

https://github.com/nasa/cFE/blob/03166722cdde3f1b337742088e7137ebacf64734/modules/es/fsw/src/cfe_es_apps.c#L606-L607

skliper avatar Oct 23 '23 15:10 skliper

This is common with static analysis, in that it doesn't know context or relationships between things. Specifically in this context the CFE ES task ID is a conversion from the OSAL task ID, and its in a context where the OSAL task ID was checked. Here's a bit more context: https://github.com/nasa/cFE/blob/03166722cdde3f1b337742088e7137ebacf64734/modules/es/fsw/src/cfe_es_apps.c#L598-L607

Although CFE_ES_LocateTaskRecordByID() can theoretically return NULL, it only does so if/when passed in an ID that's not within the valid range. But unless the CFE_ES_TaskId_FromOSAL() is broken, the ID that this returns cannot be out of range.

This is a bit of a design holdover from the days where the OSAL ID was directly used as the CFE task ID. Its no longer used directly, but there is a 1:1 mapping between them, and that mapping is implemented in the CFE_ES_TaskId_FromOSAL function. So as long as that mapping is OK and for every valid OSAL ID it returns a valid CFE ES task ID, there is no real danger here.

That's not to say its 100% robust - changing OSAL or CFE could break this - but in the current configuration it is not possible to get a NULL return fromCFE_ES_LocateTaskRecordByID() here. I am not sure, but its possible that it was checked for NULL at one point but was impossible to reach from coverage testing, thus removed.

jphickey avatar Oct 24 '23 13:10 jphickey