Skip to content

Commit 4b24eba

Browse files
dzbakerdzbaker
authored
Merge pull request #93 from chillfig/SA_jsc2_1
Fix #92, Adds static analysis comments, replaces strncpy and strlen
2 parents 2dfcc48 + 0409a4a commit 4b24eba

File tree

4 files changed

+37
-21
lines changed

4 files changed

+37
-21
lines changed

‎fsw/src/mm_app.c

+5-5
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ bool MM_LookupSymbolCmd(const CFE_SB_Buffer_t *BufPtr)
455455
/*
456456
** Check if the symbol name string is a nul string
457457
*/
458-
if (strlen(SymName) == 0)
458+
if (OS_strnlen(SymName, OS_MAX_SYM_LEN) == 0)
459459
{
460460
CFE_EVS_SendEvent(MM_SYMNAME_NUL_ERR_EID, CFE_EVS_EventType_ERROR,
461461
"NUL (empty) string specified as symbol name");
@@ -482,7 +482,7 @@ bool MM_LookupSymbolCmd(const CFE_SB_Buffer_t *BufPtr)
482482
"Symbolic address can't be resolved: Name = '%s'", SymName);
483483
}
484484

485-
} /* end strlen(CmdPtr->Payload.SymName) == 0 else */
485+
} /* end OS_strnlen(CmdPtr->Payload.SymName, OS_MAX_SYM_LEN) == 0 else */
486486

487487
return Result;
488488
}
@@ -508,7 +508,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
508508
/*
509509
** Check if the filename string is a nul string
510510
*/
511-
if (strlen(FileName) == 0)
511+
if (OS_strnlen(FileName, OS_MAX_PATH_LEN) == 0)
512512
{
513513
CFE_EVS_SendEvent(MM_SYMFILENAME_NUL_ERR_EID, CFE_EVS_EventType_ERROR,
514514
"NUL (empty) string specified as symbol dump file name");
@@ -520,7 +520,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
520520
{
521521
/* Update telemetry */
522522
MM_AppData.HkPacket.Payload.LastAction = MM_SYMTBL_SAVE;
523-
strncpy(MM_AppData.HkPacket.Payload.FileName, FileName, OS_MAX_PATH_LEN);
523+
snprintf(MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN, "%s", FileName);
524524

525525
CFE_EVS_SendEvent(MM_SYMTBL_TO_FILE_INF_EID, CFE_EVS_EventType_INFORMATION,
526526
"Symbol Table Dump to File Started: Name = '%s'", FileName);
@@ -533,7 +533,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
533533
FileName);
534534
}
535535

536-
} /* end strlen(FileName) == 0 else */
536+
} /* end OS_strnlen(FileName, OS_MAX_PATH_LEN) == 0 else */
537537

538538
return Result;
539539
}

‎fsw/src/mm_dump.c

+7-3
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ bool MM_DumpMemToFileCmd(const CFE_SB_Buffer_t *BufPtr)
318318
** Update last action statistics
319319
*/
320320
MM_AppData.HkPacket.Payload.LastAction = MM_DUMP_TO_FILE;
321-
strncpy(MM_AppData.HkPacket.Payload.FileName, FileName, OS_MAX_PATH_LEN);
321+
snprintf(MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN, "%s", FileName);
322322
MM_AppData.HkPacket.Payload.MemType = CmdPtr->Payload.MemType;
323323
MM_AppData.HkPacket.Payload.Address = SrcAddress;
324324
MM_AppData.HkPacket.Payload.BytesProcessed = CmdPtr->Payload.NumOfBytes;
@@ -512,7 +512,7 @@ bool MM_DumpInEventCmd(const CFE_SB_Buffer_t *BufPtr)
512512
*/
513513
CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], HeaderString, NULL,
514514
sizeof(EventString) - EventStringTotalLength, sizeof(HeaderString));
515-
EventStringTotalLength = strlen(EventString);
515+
EventStringTotalLength = OS_strnlen(EventString, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH);
516516

517517
/*
518518
** Build dump data string
@@ -522,17 +522,21 @@ bool MM_DumpInEventCmd(const CFE_SB_Buffer_t *BufPtr)
522522
BytePtr = (uint8 *)DumpBuffer;
523523
for (i = 0; i < CmdPtr->Payload.NumOfBytes; i++)
524524
{
525+
/* SAD: No need to check snprintf return; CFE_SB_MessageStringGet() handles safe concatenation and
526+
* prevents overflow */
525527
snprintf(TempString, MM_DUMPINEVENT_TEMP_CHARS, "0x%02X ", *BytePtr);
526528
CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], TempString, NULL,
527529
sizeof(EventString) - EventStringTotalLength, sizeof(TempString));
528-
EventStringTotalLength = strlen(EventString);
530+
EventStringTotalLength = OS_strnlen(EventString, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH);
529531
BytePtr++;
530532
}
531533

532534
/*
533535
** Append tail
534536
** This adds up to 33 characters depending on pointer representation including the NUL terminator
535537
*/
538+
/* SAD: No need to check snprintf return; CFE_SB_MessageStringGet() handles safe concatenation and
539+
* prevents overflow */
536540
snprintf(TempString, MM_DUMPINEVENT_TEMP_CHARS, "from address: %p", (void *)SrcAddress);
537541
CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], TempString, NULL,
538542
sizeof(EventString) - EventStringTotalLength, sizeof(TempString));

‎fsw/src/mm_utils.c

+6-1
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
348348
MaxSize = MM_MAX_FILL_DATA_RAM;
349349
}
350350
PSP_MemType = CFE_PSP_MEM_RAM;
351+
/* SAD: No need to check snprintf return value; buffer size can store "MEM_RAM" without overflow */
351352
snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM_RAM");
352353
break;
353354
case MM_EEPROM:
@@ -364,6 +365,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
364365
MaxSize = MM_MAX_FILL_DATA_EEPROM;
365366
}
366367
PSP_MemType = CFE_PSP_MEM_EEPROM;
368+
/* SAD: No need to check snprintf return value; buffer size can store "MEM_EEPROM" without overflow */
367369
snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM_EEPROM");
368370
break;
369371
#ifdef MM_OPT_CODE_MEM32_MEMTYPE
@@ -381,6 +383,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
381383
MaxSize = MM_MAX_FILL_DATA_MEM32;
382384
}
383385
PSP_MemType = CFE_PSP_MEM_RAM;
386+
/* SAD: No need to check snprintf return value; buffer size can store "MEM32" without overflow */
384387
snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM32");
385388
if (MM_Verify32Aligned(Address, SizeInBytes) != true)
386389
{
@@ -406,6 +409,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
406409
MaxSize = MM_MAX_FILL_DATA_MEM16;
407410
}
408411
PSP_MemType = CFE_PSP_MEM_RAM;
412+
/* SAD: No need to check snprintf return value; buffer size can store "MEM16" without overflow */
409413
snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM16");
410414
if (MM_Verify16Aligned(Address, SizeInBytes) != true)
411415
{
@@ -431,6 +435,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
431435
MaxSize = MM_MAX_FILL_DATA_MEM8;
432436
}
433437
PSP_MemType = CFE_PSP_MEM_RAM;
438+
/* SAD: No need to check snprintf return value; buffer size can store "MEM8" without overflow */
434439
snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM8");
435440
break;
436441
#endif
@@ -515,7 +520,7 @@ bool MM_ResolveSymAddr(MM_SymAddr_t *SymAddr, cpuaddr *ResolvedAddr)
515520
** If the symbol name string is a nul string
516521
** we use the offset as the absolute address
517522
*/
518-
if (strlen(SymAddr->SymName) == 0)
523+
if (OS_strnlen(SymAddr->SymName, OS_MAX_SYM_LEN) == 0)
519524
{
520525
*ResolvedAddr = SymAddr->Offset;
521526
Valid = true;

‎unit-test/mm_app_tests.c

+19-12
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,8 @@ void MM_AppPipe_Test_NoopSuccess(void)
451451
MM_AppPipe(&UT_CmdBuf.Buf);
452452

453453
/* Verify results */
454-
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_NOOP, "MM_AppData.HkPacket.Payload.LastAction == MM_NOOP");
454+
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_NOOP,
455+
"MM_AppData.HkPacket.Payload.LastAction == MM_NOOP");
455456
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 1);
456457
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
457458

@@ -518,7 +519,8 @@ void MM_AppPipe_Test_ResetSuccess(void)
518519
MM_AppPipe(&UT_CmdBuf.Buf);
519520

520521
/* Verify results */
521-
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_RESET, "MM_AppData.HkPacket.Payload.LastAction == MM_RESET");
522+
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_RESET,
523+
"MM_AppData.HkPacket.Payload.LastAction == MM_RESET");
522524
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
523525
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
524526

@@ -565,8 +567,8 @@ void MM_AppPipe_Test_ResetFail(void)
565567

566568
void MM_AppPipe_Test_PeekSuccess(void)
567569
{
568-
CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
569-
CFE_MSG_FcnCode_t FcnCode = MM_PEEK_CC;
570+
CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
571+
CFE_MSG_FcnCode_t FcnCode = MM_PEEK_CC;
570572

571573
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false);
572574
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetFcnCode), &FcnCode, sizeof(FcnCode), false);
@@ -585,8 +587,8 @@ void MM_AppPipe_Test_PeekSuccess(void)
585587

586588
void MM_AppPipe_Test_PeekFail(void)
587589
{
588-
CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
589-
CFE_MSG_FcnCode_t FcnCode = MM_PEEK_CC;
590+
CFE_SB_MsgId_t TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
591+
CFE_MSG_FcnCode_t FcnCode = MM_PEEK_CC;
590592

591593
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false);
592594
UT_SetDataBuffer(UT_KEY(CFE_MSG_GetFcnCode), &FcnCode, sizeof(FcnCode), false);
@@ -1183,8 +1185,9 @@ void MM_HousekeepingCmd_Test(void)
11831185
UtAssert_True(MM_AppData.HkPacket.Payload.DataValue == 6, "MM_AppData.HkPacket.Payload.DataValue == 6");
11841186
UtAssert_True(MM_AppData.HkPacket.Payload.BytesProcessed == 7, "MM_AppData.HkPacket.Payload.BytesProcessed == 7");
11851187

1186-
UtAssert_True(strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0,
1187-
"strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0");
1188+
UtAssert_True(
1189+
strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0,
1190+
"strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0");
11881191

11891192
call_count_CFE_EVS_SendEvent = UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent));
11901193

@@ -1218,7 +1221,8 @@ void MM_LookupSymbolCmd_Test_Nominal(void)
12181221
MM_LookupSymbolCmd(&UT_CmdBuf.Buf);
12191222

12201223
/* Verify results */
1221-
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP, "MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP");
1224+
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP,
1225+
"MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP");
12221226
UtAssert_True(MM_AppData.HkPacket.Payload.Address == 0, "MM_AppData.HkPacket.Payload.Address == 0");
12231227
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
12241228
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
@@ -1345,9 +1349,12 @@ void MM_SymTblToFileCmd_Test_Nominal(void)
13451349
MM_SymTblToFileCmd(&UT_CmdBuf.Buf);
13461350

13471351
/* Verify results */
1348-
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE, "MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE");
1349-
UtAssert_True(strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0,
1350-
"strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0");
1352+
UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE,
1353+
"MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE");
1354+
UtAssert_True(
1355+
strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0,
1356+
"strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == "
1357+
"0");
13511358
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
13521359
UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
13531360

0 commit comments

Comments
 (0)