Skip to content

Commit

Permalink
Fix #93, Replaces strncpy and strlen
Browse files Browse the repository at this point in the history 
This commit addresses issues flagged during static analysis by:
- Replacing strncpy with snprintf to enhance safety and compliance.
- Replacing strlen with OS_strnlen to chance safety and compliance.
  • Loading branch information
jdfiguer authored and jdfiguer committed Jun 21, 2024
1 parent 69fc1b9 commit 851aaac
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 20 deletions.
8 changes: 4 additions & 4 deletions fsw/src/cs_compute.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,20 +536,20 @@ void CS_RecomputeEepromMemoryChildTask(void)

if (Table == CS_EEPROM_TABLE)
{
strncpy(TableType, "EEPROM", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "EEPROM");
}
if (Table == CS_MEMORY_TABLE)
{
strncpy(TableType, "Memory", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Memory");
}
if (Table == CS_CFECORE)
{
strncpy(TableType, "cFE Core", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "cFE Core");
CS_AppData.HkPacket.Payload.CfeCoreBaseline = NewChecksumValue;
}
if (Table == CS_OSCORE)
{
strncpy(TableType, "OS", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "OS");
CS_AppData.HkPacket.Payload.OSBaseline = NewChecksumValue;
}

Expand Down
2 changes: 1 addition & 1 deletion fsw/src/cs_init.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ CFE_Status_t CS_SbInit(void)
CFE_Status_t Result = CFE_SUCCESS;

/* Initialize app configuration data */
strncpy(CS_AppData.PipeName, CS_CMD_PIPE_NAME, CS_CMD_PIPE_NAME_LEN);
snprintf(CS_AppData.PipeName, sizeof(CS_AppData.PipeName), "%s", CS_CMD_PIPE_NAME);

CS_AppData.PipeDepth = CS_PIPE_DEPTH;

Expand Down
30 changes: 15 additions & 15 deletions fsw/src/cs_table_processing.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ CFE_Status_t CS_ValidateTablesChecksumDefinitionTable(void *TblPtr)
StateField = OuterEntry->State;

/* Check for non-zero length for table name */
if (strlen(OuterEntry->Name) != 0)
if (OS_strnlen(OuterEntry->Name, CFE_TBL_MAX_FULL_NAME_LEN) != 0)
{
/* Verify valid state definition */
if (((StateField == CS_STATE_EMPTY) || (StateField == CS_STATE_ENABLED) ||
Expand Down Expand Up @@ -357,7 +357,7 @@ CFE_Status_t CS_ValidateAppChecksumDefinitionTable(void *TblPtr)
}
BadCount++;
}
else if (strlen(OuterEntry->Name) != 0)
else if (OS_strnlen(OuterEntry->Name, CFE_TBL_MAX_FULL_NAME_LEN) != 0)
{
/* Verify valid state definition */
if (((StateField == CS_STATE_EMPTY) || (StateField == CS_STATE_ENABLED) ||
Expand Down Expand Up @@ -466,7 +466,7 @@ void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_En
memcpy(&StartOfResultsTable, ResultsTblPtr, sizeof(StartOfResultsTable));
memcpy(&StartOfDefTable, DefinitionTblPtr, sizeof(StartOfDefTable));

strncpy(&TableType[0], "Undef Tbl", CS_TABLETYPE_NAME_SIZE); /* Init the table type string */
snprintf(&TableType[0], CS_TABLETYPE_NAME_SIZE, "%s", "Undef Tbl"); /* Init the table type string */

/* We don't want to be doing chekcksums while changing the table out */
if (Table == CS_EEPROM_TABLE)
Expand Down Expand Up @@ -528,11 +528,11 @@ void CS_ProcessNewEepromMemoryDefinitionTable(const CS_Def_EepromMemory_Table_En
{
if (Table == CS_EEPROM_TABLE)
{
strncpy(&TableType[0], "EEPROM", CS_TABLETYPE_NAME_SIZE);
snprintf(&TableType[0], CS_TABLETYPE_NAME_SIZE, "%s", "EEPROM");
}
if (Table == CS_MEMORY_TABLE)
{
strncpy(&TableType[0], "Memory", CS_TABLETYPE_NAME_SIZE);
snprintf(&TableType[0], CS_TABLETYPE_NAME_SIZE, "%s", "Memory");
}

CFE_EVS_SendEvent(CS_PROCESS_EEPROM_MEMORY_NO_ENTRIES_INF_EID, CFE_EVS_EventType_INFORMATION,
Expand Down Expand Up @@ -825,7 +825,7 @@ CFE_Status_t CS_TableInit(CFE_TBL_Handle_t *DefinitionTableHandle, CFE_TBL_Handl
osal_id_t Fd = OS_OBJECT_ID_UNDEFINED;
char TableType[CS_TABLETYPE_NAME_SIZE];

strncpy(TableType, "Undef Tbl", CS_TABLETYPE_NAME_SIZE); /* Init table type */
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Undef Tbl"); /* Init table type */

SizeOfTable = NumEntries * SizeofResultsTableEntry;

Expand Down Expand Up @@ -904,19 +904,19 @@ CFE_Status_t CS_TableInit(CFE_TBL_Handle_t *DefinitionTableHandle, CFE_TBL_Handl
{
if (Table == CS_EEPROM_TABLE)
{
strncpy(TableType, "EEPROM", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "EEPROM");
}
if (Table == CS_MEMORY_TABLE)
{
strncpy(TableType, "Memory", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Memory");
}
if (Table == CS_TABLES_TABLE)
{
strncpy(TableType, "Tables", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Tables");
}
if (Table == CS_APP_TABLE)
{
strncpy(TableType, "Apps", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Apps");
}

CFE_EVS_SendEvent(CS_TBL_INIT_ERR_EID, CFE_EVS_EventType_ERROR,
Expand Down Expand Up @@ -967,7 +967,7 @@ CFE_Status_t CS_HandleTableUpdate(void *DefinitionTblPtr, void *ResultsTblPtr, C
int32 Loop = 0;
char TableType[CS_TABLETYPE_NAME_SIZE];

strncpy(TableType, "Undef Tbl", CS_TABLETYPE_NAME_SIZE); /* Init table type */
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Undef Tbl"); /* Init table type */

/* Below, there are several values that are returned and assigned, but never evaluated. */
/* This is done so intentionally, as it helps us with Source-Level debugging this functions. */
Expand Down Expand Up @@ -1031,19 +1031,19 @@ CFE_Status_t CS_HandleTableUpdate(void *DefinitionTblPtr, void *ResultsTblPtr, C
{
if (Table == CS_EEPROM_TABLE)
{
strncpy(TableType, "EEPROM", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "EEPROM");
}
if (Table == CS_MEMORY_TABLE)
{
strncpy(TableType, "Memory", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Memory");
}
if (Table == CS_TABLES_TABLE)
{
strncpy(TableType, "Table", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "Table");
}
if (Table == CS_APP_TABLE)
{
strncpy(TableType, "App", CS_TABLETYPE_NAME_SIZE);
snprintf(TableType, CS_TABLETYPE_NAME_SIZE, "%s", "App");
}

/* There was a problem somewhere, generate an event */
Expand Down
49 changes: 49 additions & 0 deletions unit-test/cs_table_processing_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,8 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_Nominal(void)

strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -636,6 +638,9 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_DuplicateNameStateEmpty(void)
strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -685,6 +690,9 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_DuplicateNameStateEnabled(voi
strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -734,6 +742,9 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_DuplicateNameStateDisabled(vo
strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -779,6 +790,8 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_IllegalStateField(void)

strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -872,6 +885,10 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_TableErrorResult(void)
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[2].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(CS_AppData.DefTablesTblPtr[2].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -921,6 +938,10 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_UndefTableErrorResult(void)
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[2].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(CS_AppData.DefTablesTblPtr[2].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -968,6 +989,9 @@ void CS_ValidateTablesChecksumDefinitionTable_Test_CsTableError(void)
strncpy(CS_AppData.DefTablesTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefTablesTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefTablesTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefTablesTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateTablesChecksumDefinitionTable(CS_AppData.DefTablesTblPtr);

Expand Down Expand Up @@ -1003,6 +1027,9 @@ void CS_ValidateAppChecksumDefinitionTable_Test_Nominal(void)
CS_AppData.DefAppTblPtr[1].State = CS_STATE_ENABLED;
strncpy(CS_AppData.DefAppTblPtr[1].Name, "app2", sizeof(CS_AppData.DefAppTblPtr[1].Name));

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));

/* Execute the function being tested */
UtAssert_INT32_EQ(CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr), CFE_SUCCESS);

Expand All @@ -1029,6 +1056,9 @@ void CS_ValidateAppChecksumDefinitionTable_Test_DuplicateNameStateEmpty(void)
strncpy(CS_AppData.DefAppTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefAppTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down Expand Up @@ -1078,6 +1108,9 @@ void CS_ValidateAppChecksumDefinitionTable_Test_DuplicateNameStateEnabled(void)
strncpy(CS_AppData.DefAppTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefAppTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down Expand Up @@ -1127,6 +1160,9 @@ void CS_ValidateAppChecksumDefinitionTable_Test_DuplicateNameStateDisabled(void)
strncpy(CS_AppData.DefAppTblPtr[0].Name, "name", 10);
strncpy(CS_AppData.DefAppTblPtr[1].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down Expand Up @@ -1172,6 +1208,8 @@ void CS_ValidateAppChecksumDefinitionTable_Test_IllegalStateField(void)

strncpy(CS_AppData.DefAppTblPtr[0].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down Expand Up @@ -1249,6 +1287,9 @@ void CS_ValidateAppChecksumDefinitionTable_Test_LongName(void)
memset(CS_AppData.DefAppTblPtr[0].Name, 'x', sizeof(CS_AppData.DefAppTblPtr[0].Name));
memset(CS_AppData.DefAppTblPtr[1].Name, 'y', sizeof(CS_AppData.DefAppTblPtr[1].Name));

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));

UtAssert_INT32_EQ(CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr), CS_TABLE_ERROR);

/* Verify results */
Expand Down Expand Up @@ -1280,6 +1321,10 @@ void CS_ValidateAppChecksumDefinitionTable_Test_TableErrorResult(void)
strncpy(CS_AppData.DefAppTblPtr[1].Name, "name", 10);
strncpy(CS_AppData.DefAppTblPtr[2].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(CS_AppData.DefAppTblPtr[2].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down Expand Up @@ -1328,6 +1373,10 @@ void CS_ValidateAppChecksumDefinitionTable_Test_UndefTableErrorResult(void)
strncpy(CS_AppData.DefAppTblPtr[1].Name, "name", 10);
strncpy(CS_AppData.DefAppTblPtr[2].Name, "name", 10);

UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(CS_AppData.DefAppTblPtr[0].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 2, strlen(CS_AppData.DefAppTblPtr[1].Name));
UT_SetDeferredRetcode(UT_KEY(OS_strnlen), 3, strlen(CS_AppData.DefAppTblPtr[2].Name));

/* Execute the function being tested */
Result = CS_ValidateAppChecksumDefinitionTable(CS_AppData.DefAppTblPtr);

Expand Down

0 comments on commit 851aaac

Please sign in to comment.