Fix nasa#92, Adds static analysis comments, replaces strncpy and strlen

This commit addresses issues flagged during static analysis by:
- Adding JSC 2.1 disposition comments.
- Replacing strncpy with snprintf to enhance safety and compliance.
- Replacing strlen with OS_strnlen.

fsw/src/mm_app.c

@@ -455,7 +455,7 @@ bool MM_LookupSymbolCmd(const CFE_SB_Buffer_t *BufPtr)
     /*
     ** Check if the symbol name string is a nul string
     */
-    if (strlen(SymName) == 0)
+    if (OS_strnlen(SymName, OS_MAX_SYM_LEN) == 0)
     {
         CFE_EVS_SendEvent(MM_SYMNAME_NUL_ERR_EID, CFE_EVS_EventType_ERROR,
                           "NUL (empty) string specified as symbol name");
@@ -482,7 +482,7 @@ bool MM_LookupSymbolCmd(const CFE_SB_Buffer_t *BufPtr)
                               "Symbolic address can't be resolved: Name = '%s'", SymName);
         }
 
-    } /* end strlen(CmdPtr->Payload.SymName) == 0 else */
+    } /* end OS_strnlen(CmdPtr->Payload.SymName, OS_MAX_SYM_LEN) == 0 else */
 
     return Result;
 }
@@ -508,7 +508,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
     /*
     ** Check if the filename string is a nul string
     */
-    if (strlen(FileName) == 0)
+    if (OS_strnlen(FileName, OS_MAX_PATH_LEN) == 0)
     {
         CFE_EVS_SendEvent(MM_SYMFILENAME_NUL_ERR_EID, CFE_EVS_EventType_ERROR,
                           "NUL (empty) string specified as symbol dump file name");
@@ -520,7 +520,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
         {
             /* Update telemetry */
             MM_AppData.HkPacket.Payload.LastAction = MM_SYMTBL_SAVE;
-            strncpy(MM_AppData.HkPacket.Payload.FileName, FileName, OS_MAX_PATH_LEN);
+            snprintf(MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN, "%s", FileName);
 
             CFE_EVS_SendEvent(MM_SYMTBL_TO_FILE_INF_EID, CFE_EVS_EventType_INFORMATION,
                               "Symbol Table Dump to File Started: Name = '%s'", FileName);
@@ -533,7 +533,7 @@ bool MM_SymTblToFileCmd(const CFE_SB_Buffer_t *BufPtr)
                               FileName);
         }
 
-    } /* end strlen(FileName) == 0 else */
+    } /* end OS_strnlen(FileName, OS_MAX_PATH_LEN) == 0 else */
 
     return Result;
 }

fsw/src/mm_dump.c

@@ -318,7 +318,7 @@ bool MM_DumpMemToFileCmd(const CFE_SB_Buffer_t *BufPtr)
                         ** Update last action statistics
                         */
                         MM_AppData.HkPacket.Payload.LastAction = MM_DUMP_TO_FILE;
-                        strncpy(MM_AppData.HkPacket.Payload.FileName, FileName, OS_MAX_PATH_LEN);
+                        snprintf(MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN, "%s", FileName);
                         MM_AppData.HkPacket.Payload.MemType        = CmdPtr->Payload.MemType;
                         MM_AppData.HkPacket.Payload.Address        = SrcAddress;
                         MM_AppData.HkPacket.Payload.BytesProcessed = CmdPtr->Payload.NumOfBytes;
@@ -512,7 +512,7 @@ bool MM_DumpInEventCmd(const CFE_SB_Buffer_t *BufPtr)
                 */
                 CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], HeaderString, NULL,
                                         sizeof(EventString) - EventStringTotalLength, sizeof(HeaderString));
-                EventStringTotalLength = strlen(EventString);
+                EventStringTotalLength = OS_strnlen(EventString, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH);
 
                 /*
                 ** Build dump data string
@@ -522,17 +522,21 @@ bool MM_DumpInEventCmd(const CFE_SB_Buffer_t *BufPtr)
                 BytePtr = (uint8 *)DumpBuffer;
                 for (i = 0; i < CmdPtr->Payload.NumOfBytes; i++)
                 {
+                    /* SAD: No need to check snprintf return; CFE_SB_MessageStringGet() handles safe concatenation and
+                     * prevents overflow */
                     snprintf(TempString, MM_DUMPINEVENT_TEMP_CHARS, "0x%02X ", *BytePtr);
                     CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], TempString, NULL,
                                             sizeof(EventString) - EventStringTotalLength, sizeof(TempString));
-                    EventStringTotalLength = strlen(EventString);
+                    EventStringTotalLength = OS_strnlen(EventString, CFE_MISSION_EVS_MAX_MESSAGE_LENGTH);
                     BytePtr++;
                 }
 
                 /*
                 ** Append tail
                 ** This adds up to 33 characters depending on pointer representation including the NUL terminator
                 */
+                /* SAD: No need to check snprintf return; CFE_SB_MessageStringGet() handles safe concatenation and
+                 * prevents overflow */
                 snprintf(TempString, MM_DUMPINEVENT_TEMP_CHARS, "from address: %p", (void *)SrcAddress);
                 CFE_SB_MessageStringGet(&EventString[EventStringTotalLength], TempString, NULL,
                                         sizeof(EventString) - EventStringTotalLength, sizeof(TempString));

fsw/src/mm_utils.c

@@ -348,6 +348,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
                     MaxSize = MM_MAX_FILL_DATA_RAM;
                 }
                 PSP_MemType = CFE_PSP_MEM_RAM;
+                /* SAD: No need to check snprintf return value; buffer size can store "MEM_RAM" without overflow */
                 snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM_RAM");
                 break;
             case MM_EEPROM:
@@ -364,6 +365,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
                     MaxSize = MM_MAX_FILL_DATA_EEPROM;
                 }
                 PSP_MemType = CFE_PSP_MEM_EEPROM;
+                /* SAD: No need to check snprintf return value; buffer size can store "MEM_EEPROM" without overflow */
                 snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM_EEPROM");
                 break;
 #ifdef MM_OPT_CODE_MEM32_MEMTYPE
@@ -381,6 +383,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
                     MaxSize = MM_MAX_FILL_DATA_MEM32;
                 }
                 PSP_MemType = CFE_PSP_MEM_RAM;
+                /* SAD: No need to check snprintf return value; buffer size can store "MEM32" without overflow */
                 snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM32");
                 if (MM_Verify32Aligned(Address, SizeInBytes) != true)
                 {
@@ -406,6 +409,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
                     MaxSize = MM_MAX_FILL_DATA_MEM16;
                 }
                 PSP_MemType = CFE_PSP_MEM_RAM;
+                /* SAD: No need to check snprintf return value; buffer size can store "MEM16" without overflow */
                 snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM16");
                 if (MM_Verify16Aligned(Address, SizeInBytes) != true)
                 {
@@ -431,6 +435,7 @@ bool MM_VerifyLoadDumpParams(cpuaddr Address, MM_MemType_t MemType, size_t SizeI
                     MaxSize = MM_MAX_FILL_DATA_MEM8;
                 }
                 PSP_MemType = CFE_PSP_MEM_RAM;
+                /* SAD: No need to check snprintf return value; buffer size can store "MEM8" without overflow */
                 snprintf(MemTypeStr, MM_MAX_MEM_TYPE_STR_LEN, "%s", "MEM8");
                 break;
 #endif
@@ -515,7 +520,7 @@ bool MM_ResolveSymAddr(MM_SymAddr_t *SymAddr, cpuaddr *ResolvedAddr)
     ** If the symbol name string is a nul string
     ** we use the offset as the absolute address
     */
-    if (strlen(SymAddr->SymName) == 0)
+    if (OS_strnlen(SymAddr->SymName, OS_MAX_SYM_LEN) == 0)
     {
         *ResolvedAddr = SymAddr->Offset;
         Valid         = true;

unit-test/mm_app_tests.c

@@ -451,7 +451,8 @@ void MM_AppPipe_Test_NoopSuccess(void)
     MM_AppPipe(&UT_CmdBuf.Buf);
 
     /* Verify results */
-    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_NOOP, "MM_AppData.HkPacket.Payload.LastAction == MM_NOOP");
+    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_NOOP,
+                  "MM_AppData.HkPacket.Payload.LastAction == MM_NOOP");
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 1);
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
 
@@ -518,7 +519,8 @@ void MM_AppPipe_Test_ResetSuccess(void)
     MM_AppPipe(&UT_CmdBuf.Buf);
 
     /* Verify results */
-    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_RESET, "MM_AppData.HkPacket.Payload.LastAction == MM_RESET");
+    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_RESET,
+                  "MM_AppData.HkPacket.Payload.LastAction == MM_RESET");
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
 
@@ -565,8 +567,8 @@ void MM_AppPipe_Test_ResetFail(void)
 
 void MM_AppPipe_Test_PeekSuccess(void)
 {
-    CFE_SB_MsgId_t    TestMsgId             = CFE_SB_ValueToMsgId(MM_CMD_MID);
-    CFE_MSG_FcnCode_t FcnCode               = MM_PEEK_CC;
+    CFE_SB_MsgId_t    TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
+    CFE_MSG_FcnCode_t FcnCode   = MM_PEEK_CC;
 
     UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false);
     UT_SetDataBuffer(UT_KEY(CFE_MSG_GetFcnCode), &FcnCode, sizeof(FcnCode), false);
@@ -585,8 +587,8 @@ void MM_AppPipe_Test_PeekSuccess(void)
 
 void MM_AppPipe_Test_PeekFail(void)
 {
-    CFE_SB_MsgId_t    TestMsgId             = CFE_SB_ValueToMsgId(MM_CMD_MID);
-    CFE_MSG_FcnCode_t FcnCode               = MM_PEEK_CC;
+    CFE_SB_MsgId_t    TestMsgId = CFE_SB_ValueToMsgId(MM_CMD_MID);
+    CFE_MSG_FcnCode_t FcnCode   = MM_PEEK_CC;
 
     UT_SetDataBuffer(UT_KEY(CFE_MSG_GetMsgId), &TestMsgId, sizeof(TestMsgId), false);
     UT_SetDataBuffer(UT_KEY(CFE_MSG_GetFcnCode), &FcnCode, sizeof(FcnCode), false);
@@ -866,6 +868,7 @@ void MM_AppPipe_Test_LookupSymbolSuccess(void)
     UT_SetDataBuffer(UT_KEY(CFE_MSG_GetSize), &MsgSize, sizeof(MsgSize), false);
 
     strncpy(UT_CmdBuf.LookupSymCmd.Payload.SymName, "name", sizeof(UT_CmdBuf.LookupSymCmd.Payload.SymName) - 1);
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
     UT_SetDataBuffer(UT_KEY(OS_SymbolLookup), &ResolvedAddr, sizeof(ResolvedAddr), false);
 
     /* ignore dummy message length check */
@@ -922,6 +925,8 @@ void MM_AppPipe_Test_SymTblToFileSuccess(void)
 
     strncpy(UT_CmdBuf.SymTblToFileCmd.Payload.FileName, "name", sizeof(UT_CmdBuf.SymTblToFileCmd.Payload.FileName) - 1);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
+
     /* Set to satisfy condition "OS_Status == OS_SUCCESS" */
     UT_SetDeferredRetcode(UT_KEY(OS_SymbolTableDump), 1, CFE_SUCCESS);
 
@@ -1183,8 +1188,9 @@ void MM_HousekeepingCmd_Test(void)
     UtAssert_True(MM_AppData.HkPacket.Payload.DataValue == 6, "MM_AppData.HkPacket.Payload.DataValue == 6");
     UtAssert_True(MM_AppData.HkPacket.Payload.BytesProcessed == 7, "MM_AppData.HkPacket.Payload.BytesProcessed == 7");
 
-    UtAssert_True(strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0,
-                  "strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0");
+    UtAssert_True(
+        strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0,
+        "strncmp(MM_AppData.HkPacket.Payload.FileName, MM_AppData.HkPacket.Payload.FileName, OS_MAX_PATH_LEN) == 0");
 
     call_count_CFE_EVS_SendEvent = UT_GetStubCount(UT_KEY(CFE_EVS_SendEvent));
 
@@ -1209,6 +1215,8 @@ void MM_LookupSymbolCmd_Test_Nominal(void)
 
     strncpy(UT_CmdBuf.LookupSymCmd.Payload.SymName, "name", sizeof(UT_CmdBuf.LookupSymCmd.Payload.SymName) - 1);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
+
     /* ignore dummy message length check */
     UT_SetDefaultReturnValue(UT_KEY(MM_VerifyCmdLength), true);
 
@@ -1218,7 +1226,8 @@ void MM_LookupSymbolCmd_Test_Nominal(void)
     MM_LookupSymbolCmd(&UT_CmdBuf.Buf);
 
     /* Verify results */
-    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP, "MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP");
+    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP,
+                  "MM_AppData.HkPacket.Payload.LastAction == MM_SYM_LOOKUP");
     UtAssert_True(MM_AppData.HkPacket.Payload.Address == 0, "MM_AppData.HkPacket.Payload.Address == 0");
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
@@ -1292,6 +1301,8 @@ void MM_LookupSymbolCmd_Test_SymbolLookupError(void)
 
     strncpy(UT_CmdBuf.LookupSymCmd.Payload.SymName, "name", sizeof(UT_CmdBuf.LookupSymCmd.Payload.SymName) - 1);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
+
     /* Set to generate error message MM_SYMNAME_ERR_EID */
     UT_SetDeferredRetcode(UT_KEY(OS_SymbolLookup), 1, -1);
 
@@ -1335,6 +1346,8 @@ void MM_SymTblToFileCmd_Test_Nominal(void)
 
     strncpy(UT_CmdBuf.SymTblToFileCmd.Payload.FileName, "name", sizeof(UT_CmdBuf.SymTblToFileCmd.Payload.FileName) - 1);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
+
     /* Set to satisfy condition "OS_Status == OS_SUCCESS" */
     UT_SetDeferredRetcode(UT_KEY(OS_SymbolTableDump), 1, CFE_SUCCESS);
 
@@ -1345,9 +1358,12 @@ void MM_SymTblToFileCmd_Test_Nominal(void)
     MM_SymTblToFileCmd(&UT_CmdBuf.Buf);
 
     /* Verify results */
-    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE, "MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE");
-    UtAssert_True(strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0,
-                  "strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0");
+    UtAssert_True(MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE,
+                  "MM_AppData.HkPacket.Payload.LastAction == MM_SYMTBL_SAVE");
+    UtAssert_True(
+        strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == 0,
+        "strncmp(MM_AppData.HkPacket.Payload.FileName, UT_CmdBuf.SymTblToFileCmd.Payload.FileName, OS_MAX_PATH_LEN) == "
+        "0");
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.CmdCounter, 0);
     UtAssert_INT32_EQ(MM_AppData.HkPacket.Payload.ErrCounter, 0);
 
@@ -1421,6 +1437,8 @@ void MM_SymTblToFileCmd_Test_SymbolTableDumpError(void)
 
     strncpy(UT_CmdBuf.SymTblToFileCmd.Payload.FileName, "name", sizeof(UT_CmdBuf.SymTblToFileCmd.Payload.FileName) - 1);
 
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), sizeof("name"));
+
     /* Set to generate error message MM_SYMTBL_TO_FILE_FAIL_ERR_EID */
     UT_SetDeferredRetcode(UT_KEY(OS_SymbolTableDump), 1, -1);
 

unit-test/mm_utils_tests.c

@@ -3514,6 +3514,7 @@ void MM_ResolveSymAddr_Test(void)
 
     UT_SetDataBuffer(UT_KEY(OS_SymbolLookup), &ResolvedAddr, sizeof(ResolvedAddr), false);
     UT_SetDefaultReturnValue(UT_KEY(OS_SymbolLookup), OS_SUCCESS);
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(SymAddr.SymName));
 
     /* Execute the function being tested */
     Result = MM_ResolveSymAddr(&SymAddr, &ResolvedAddr);
@@ -3523,6 +3524,7 @@ void MM_ResolveSymAddr_Test(void)
     UtAssert_True(ResolvedAddr == SymAddr.Offset, "ResolvedAddr == SymAddr.Offset");
 
     UT_SetDefaultReturnValue(UT_KEY(OS_SymbolLookup), -1);
+    UT_SetDefaultReturnValue(UT_KEY(OS_strnlen), strlen(SymAddr.SymName));
 
     /* Execute the function being tested */
     Result = MM_ResolveSymAddr(&SymAddr, &ResolvedAddr);

unit-test/stubs/mm_utils_stubs.c

@@ -99,4 +99,4 @@ int32 MM_ComputeCRCFromFile(osal_id_t FileHandle, uint32 *CrcPtr, uint32 TypeCRC
     UT_Stub_RegisterContext(UT_KEY(MM_ComputeCRCFromFile), CrcPtr);
     UT_Stub_RegisterContextGenericArg(UT_KEY(MM_ComputeCRCFromFile), TypeCRC);
     return UT_DEFAULT_IMPL(MM_ComputeCRCFromFile);
-}
+}