Fix nasa#117, Replaces strncpy with snprintf

This commit addresses issues flagged during static analysis by:
- Replacing strncpy with snprintf to enhance safety and compliance.
- Replacing strlen with FM_strnlen.

fsw/src/fm_child.c

@@ -801,8 +801,7 @@ void FM_ChildFileInfoCmd(FM_ChildQueueEntry_t *CmdArgs)
 
     /* Report directory or filename state, name, size and time */
     ReportPtr->FileStatus = (uint8)CmdArgs->FileInfoState;
-    strncpy(ReportPtr->Filename, CmdArgs->Source1, OS_MAX_PATH_LEN - 1);
-    ReportPtr->Filename[OS_MAX_PATH_LEN - 1] = '\0';
+    snprintf(ReportPtr->Filename, OS_MAX_PATH_LEN, "%s", CmdArgs->Source1);
 
     ReportPtr->FileSize         = CmdArgs->FileInfoSize;
     ReportPtr->LastModifiedTime = CmdArgs->FileInfoTime;
@@ -1136,7 +1135,7 @@ void FM_ChildDirListPktCmd(const FM_ChildQueueEntry_t *CmdArgs)
     **  CmdArgs->Source2       = directory name plus separator
     **  CmdArgs->DirListOffset = index of 1st reported dir entry
     */
-    PathLength = strlen(CmdArgs->Source2);
+    PathLength = FM_strnlen(CmdArgs->Source2, OS_MAX_PATH_LEN);
 
     /* Open source directory for reading directory list */
     Status = OS_DirectoryOpen(&DirId, CmdArgs->Source1);
@@ -1157,9 +1156,8 @@ void FM_ChildDirListPktCmd(const FM_ChildQueueEntry_t *CmdArgs)
 
         ReportPtr = &FM_GlobalData.DirListPkt.Payload;
 
-        strncpy(ReportPtr->DirName, CmdArgs->Source1, OS_MAX_PATH_LEN - 1);
-        ReportPtr->DirName[OS_MAX_PATH_LEN - 1] = '\0';
-        ReportPtr->FirstFile                    = CmdArgs->DirListOffset;
+        snprintf(ReportPtr->DirName, OS_MAX_PATH_LEN, "%s", CmdArgs->Source1);
+        ReportPtr->FirstFile = CmdArgs->DirListOffset;
 
         StillProcessing = true;
         while (StillProcessing == true)
@@ -1187,14 +1185,13 @@ void FM_ChildDirListPktCmd(const FM_ChildQueueEntry_t *CmdArgs)
                     ListIndex = ReportPtr->PacketFiles;
                     ListEntry = &ReportPtr->FileList[ListIndex];
 
-                    EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
+                    EntryLength = FM_strnlen(OS_DIRENTRY_NAME(DirEntry), OS_MAX_FILE_NAME);
 
                     /* Verify combined directory plus filename length */
                     if ((PathLength + EntryLength) < sizeof(LogicalName))
                     {
                         /* Add filename to directory listing telemetry packet */
-                        strncpy(ListEntry->EntryName, OS_DIRENTRY_NAME(DirEntry), sizeof(ListEntry->EntryName) - 1);
-                        ListEntry->EntryName[sizeof(ListEntry->EntryName) - 1] = '\0';
+                        snprintf(ListEntry->EntryName, sizeof(ListEntry->EntryName), "%s", OS_DIRENTRY_NAME(DirEntry));
 
                         /* Build filename - Directory already has path separator */
                         memcpy(LogicalName, CmdArgs->Source2, PathLength);
@@ -1378,7 +1375,7 @@ void FM_ChildDirListFileLoop(osal_id_t DirId, osal_id_t FileHandle, const char *
 
     memset(&DirEntry, 0, sizeof(DirEntry));
 
-    PathLength = strlen(DirWithSep);
+    PathLength = FM_strnlen(DirWithSep, OS_MAX_PATH_LEN);
 
     /* Until end of directory entries or output file write error */
     while ((CommandResult == true) && (ReadingDirectory == true))
@@ -1399,7 +1396,7 @@ void FM_ChildDirListFileLoop(osal_id_t DirId, osal_id_t FileHandle, const char *
             /* Count all files - write limited number */
             if (FileEntries < FM_DIR_LIST_FILE_ENTRIES)
             {
-                EntryLength = strlen(OS_DIRENTRY_NAME(DirEntry));
+                EntryLength = FM_strnlen(OS_DIRENTRY_NAME(DirEntry), OS_MAX_FILE_NAME);
 
                 /*
                  * DirListData.EntryName and TempName are both OS_MAX_PATH_LEN, DirEntry name is OS_MAX_FILE_NAME,
@@ -1484,9 +1481,9 @@ void FM_ChildDirListFileLoop(osal_id_t DirId, osal_id_t FileHandle, const char *
     {
         FM_GlobalData.ChildCmdCounter++;
 
-        CFE_EVS_SendEvent(FM_GET_DIR_FILE_CMD_INF_EID,
-                          CFE_EVS_EventType_INFORMATION, "%s command: wrote %d of %d names: dir = %s, filename = %s",
-                          CmdText, (int)FileEntries, (int)DirEntries, Directory, Filename);
+        CFE_EVS_SendEvent(FM_GET_DIR_FILE_CMD_INF_EID, CFE_EVS_EventType_INFORMATION,
+                          "%s command: wrote %d of %d names: dir = %s, filename = %s", CmdText, (int)FileEntries,
+                          (int)DirEntries, Directory, Filename);
     }
 }
 

fsw/src/fm_cmd_utils.c

@@ -508,7 +508,7 @@ void FM_AppendPathSep(char *Directory, uint32 BufferSize)
     */
     size_t StringLength = 0;
 
-    StringLength = strlen(Directory);
+    StringLength = FM_strnlen(Directory, OS_MAX_PATH_LEN);
 
     /* Do nothing if string already ends with a path separator */
     if ((StringLength != 0) && (Directory[StringLength - 1] != '/'))
@@ -577,9 +577,8 @@ CFE_Status_t FM_GetDirectorySpaceEstimate(const char *Directory, uint64 *BlockCo
     TotalBytes = 0;
 
     memset(&DirEntry, 0, sizeof(DirEntry));
-    strncpy(FullPath, Directory, sizeof(FullPath) - 1);
-    FullPath[sizeof(FullPath) - 1] = 0;
-    DirLen                         = strlen(FullPath);
+    snprintf(FullPath, sizeof(FullPath), "%s", Directory);
+    DirLen = FM_strnlen(FullPath, OS_MAX_PATH_LEN);
     if (DirLen < (sizeof(FullPath) - 2))
     {
         FullPath[DirLen] = '/';
@@ -607,7 +606,7 @@ CFE_Status_t FM_GetDirectorySpaceEstimate(const char *Directory, uint64 *BlockCo
         /* Read each directory entry and stat the files */
         while (OS_DirectoryRead(DirId, &DirEntry) == OS_SUCCESS)
         {
-            strncpy(&FullPath[DirLen], OS_DIRENTRY_NAME(DirEntry), sizeof(FullPath) - DirLen - 1);
+            snprintf(&FullPath[DirLen], sizeof(FullPath) - DirLen, "%s", OS_DIRENTRY_NAME(DirEntry));
 
             OS_Status = OS_stat(FullPath, &FileStat);
             if (OS_Status != OS_SUCCESS)
@@ -635,3 +634,19 @@ CFE_Status_t FM_GetDirectorySpaceEstimate(const char *Directory, uint64 *BlockCo
 
     return Result;
 }
+
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+/*                                                                 */
+/* FM utility function -- get string length                        */
+/*                                                                 */
+/* * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * */
+size_t FM_strnlen(const char *str, size_t maxlen)
+{
+    const char *end = memchr(str, 0, maxlen);
+    if (end != NULL)
+    {
+        /* actual length of string is difference */
+        maxlen = end - str;
+    }
+    return maxlen;
+}

fsw/src/fm_cmd_utils.h

@@ -400,4 +400,21 @@ CFE_Status_t FM_GetVolumeFreeSpace(const char *FileSys, uint64 *BlockCount, uint
  */
 CFE_Status_t FM_GetDirectorySpaceEstimate(const char *Directory, uint64 *BlockCount, uint64 *ByteCount);
 
+/************************************************************************/
+/** @brief Calculates the length of a string up to a maximum length
+ *
+ * Purpose: Provides a local OSAL routine to get the functionality
+ *          of the (non-C99) "strnlen()" function, via the
+ *          C89/C99 standard "memchr()" function instead.
+ *
+ * @par Assumptions, External Events, and Notes:
+ *       None
+ *
+ * @param str     Pointer to the input string
+ * @param maxlen  Maximum number of characters to check
+ *
+ * @returns Length of the string up to `maxlen` characters
+ */
+size_t FM_strnlen(const char *str, size_t maxlen);
+
 #endif

fsw/src/fm_cmds.c

@@ -387,10 +387,8 @@ bool FM_DecompressFileCmd(const CFE_SB_Buffer_t *BufPtr)
 
         /* Set handshake queue command args */
         CmdArgs->CommandCode = FM_DECOMPRESS_FILE_CC;
-        strncpy(CmdArgs->Source1, CmdPtr->Source, OS_MAX_PATH_LEN - 1);
-        CmdArgs->Source1[OS_MAX_PATH_LEN - 1] = '\0';
-        strncpy(CmdArgs->Target, CmdPtr->Target, OS_MAX_PATH_LEN - 1);
-        CmdArgs->Target[OS_MAX_PATH_LEN - 1] = '\0';
+        snprintf(CmdArgs->Source1, OS_MAX_PATH_LEN, "%s", CmdPtr->Source);
+        snprintf(CmdArgs->Target, OS_MAX_PATH_LEN, "%s", CmdPtr->Target);
 
         /* Invoke lower priority child task */
         FM_InvokeChildTask();
@@ -831,7 +829,8 @@ bool FM_MonitorFilesystemSpaceCmd(const CFE_SB_Buffer_t *BufPtr)
         CFE_SB_TransmitMsg(CFE_MSG_PTR(FM_GlobalData.MonitorReportPkt.TelemetryHeader), true);
 
         /* Send command completion event (info) */
-        CFE_EVS_SendEvent(FM_MONITOR_FILESYSTEM_SPACE_CMD_INF_EID, CFE_EVS_EventType_INFORMATION, "%s command", CmdText);
+        CFE_EVS_SendEvent(FM_MONITOR_FILESYSTEM_SPACE_CMD_INF_EID, CFE_EVS_EventType_INFORMATION, "%s command",
+                          CmdText);
     }
 
     return CommandResult;

unit-test/fm_child_tests.c

@@ -1747,6 +1747,7 @@ void Test_FM_ChildDirListPktCmd_PathAndEntryLengthGreaterMaxPathLength(void)
 
     UT_SetDeferredRetcode(UT_KEY(OS_DirectoryRead), 2, !OS_SUCCESS);
     UT_SetDataBuffer(UT_KEY(OS_DirectoryRead), &direntry, sizeof(direntry), false);
+    UT_SetDefaultReturnValue(UT_KEY(FM_strnlen), strlen(queue_entry.Source2));
 
     /* Act */
     UtAssert_VOIDCALL(FM_ChildDirListPktCmd(&queue_entry));
@@ -1985,6 +1986,8 @@ void Test_FM_ChildDirListFileLoop_PathLengthAndEntryLengthGreaterMaxPathLen(void
 
     UT_SetDataBuffer(UT_KEY(OS_DirectoryRead), &direntry, sizeof(direntry), false);
     UT_SetDeferredRetcode(UT_KEY(OS_DirectoryRead), 2, !OS_SUCCESS);
+    UT_SetDefaultReturnValue(UT_KEY(FM_strnlen), strlen(dirwithsep));
+    UT_SetDeferredRetcode(UT_KEY(FM_strnlen), 2, strlen(OS_DIRENTRY_NAME(direntry)));
 
     /* Act */
     UtAssert_VOIDCALL(FM_ChildDirListFileLoop(FM_UT_OBJID_1, FM_UT_OBJID_2, "dir", dirwithsep, "fname", false));

unit-test/fm_cmd_utils_tests.c

@@ -669,6 +669,40 @@ void Test_FM_GetDirectorySpaceEstimate(void)
     UtAssert_STUB_COUNT(OS_DirectoryRead, 0);
 }
 
+/* **************************
+ * FM_strnlen Tests
+ * *************************/
+void Test_FM_strnlen_null_character_found(void)
+{
+    /* Arrange */
+    size_t result;
+    char   str[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH];
+
+    memset(str, 0xFF, sizeof(str) - 1);
+    str[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH - 1] = '\0';
+
+    /* Act */
+    result = FM_strnlen(str, sizeof(str));
+
+    /* Assert */
+    UtAssert_INT32_EQ(result, sizeof(str) - 1);
+}
+
+void Test_FM_strnlen_null_character_not_found(void)
+{
+    /* Arrange */
+    size_t result;
+    char   str[CFE_MISSION_EVS_MAX_MESSAGE_LENGTH];
+
+    memset(str, 0xFF, sizeof(str));
+
+    /* Act */
+    result = FM_strnlen(str, sizeof(str));
+
+    /* Assert */
+    UtAssert_INT32_EQ(result, sizeof(str));
+}
+
 /*
  * Register the test cases to execute with the unit test tool
  */
@@ -690,4 +724,8 @@ void UtTest_Setup(void)
     UtTest_Add(Test_FM_AppendPathSep, FM_Test_Setup, FM_Test_Teardown, "Test_FM_AppendPathSep");
     UtTest_Add(Test_FM_GetVolumeFreeSpace, FM_Test_Setup, FM_Test_Teardown, "Test_FM_GetVolumeFreeSpace");
     UtTest_Add(Test_FM_GetDirectorySpaceEstimate, FM_Test_Setup, FM_Test_Teardown, "Test_FM_GetDirectorySpaceEstimate");
+    UtTest_Add(Test_FM_strnlen_null_character_found, FM_Test_Setup, FM_Test_Teardown,
+               "Test_FM_strnlen_null_character_found");
+    UtTest_Add(Test_FM_strnlen_null_character_not_found, FM_Test_Setup, FM_Test_Teardown,
+               "Test_FM_strnlen_null_character_not_found");
 }

unit-test/stubs/fm_cmd_utils_stubs.c

@@ -318,3 +318,20 @@ bool FM_VerifyOverwrite(uint16 Overwrite, uint32 EventID, const char *CmdText)
 
     return UT_GenStub_GetReturnValue(FM_VerifyOverwrite, bool);
 }
+
+/*
+ * ----------------------------------------------------
+ * Generated stub function for FM_strnlen()
+ * ----------------------------------------------------
+ */
+size_t FM_strnlen(const char *str, size_t maxlen)
+{
+    UT_GenStub_SetupReturnBuffer(FM_strnlen, size_t);
+
+    UT_GenStub_AddParam(FM_strnlen, const char *, str);
+    UT_GenStub_AddParam(FM_strnlen, size_t, maxlen);
+
+    UT_GenStub_Execute(FM_strnlen, Basic, NULL);
+
+    return UT_GenStub_GetReturnValue(FM_strnlen, size_t);
+}