From f664494dcec8beb70402927524d9950bf1b1441f Mon Sep 17 00:00:00 2001 From: jdfiguer Date: Mon, 10 Jun 2024 09:35:43 -0400 Subject: [PATCH] Fix #117, Replaces strncpy and strlen This commit addresses issues flagged during static analysis by: - Replacing strncpy with snprintf to enhance safety and compliance. - Replacing strlen with OS_strnlen. --- fsw/src/fm_child.c | 25 +++++++++++-------------- fsw/src/fm_cmd_utils.c | 9 ++++----- fsw/src/fm_cmds.c | 9 ++++----- 3 files changed, 19 insertions(+), 24 deletions(-) diff --git a/fsw/src/fm_child.c b/fsw/src/fm_child.c index 8559abc..5469118 100644 --- a/fsw/src/fm_child.c +++ b/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 = OS_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 = OS_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 = OS_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 = OS_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); } } diff --git a/fsw/src/fm_cmd_utils.c b/fsw/src/fm_cmd_utils.c index 4b018bc..08f214e 100644 --- a/fsw/src/fm_cmd_utils.c +++ b/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 = OS_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 = OS_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) diff --git a/fsw/src/fm_cmds.c b/fsw/src/fm_cmds.c index 48f4ace..1420c2f 100644 --- a/fsw/src/fm_cmds.c +++ b/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;